From 16f7f8d2503a9033a09a4d7e857561d63471bb82 Mon Sep 17 00:00:00 2001
From: Yabin Cui <yabinc@google.com>
Date: Tue, 4 Nov 2014 11:08:05 -0800
Subject: [PATCH] check invalid file offset when loading library

Bug: 18178121
Bug: 18078224

Change-Id: I5254433d54645db68e9b83d5095dc2bf9d8531bc
---
 linker/linker.cpp    |  8 ++++++++
 tests/dlext_test.cpp | 22 ++++++++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/linker/linker.cpp b/linker/linker.cpp
index ab0fc0762..b2911b8ce 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -814,12 +814,20 @@ static soinfo* load_library(LoadTaskList& load_tasks, const char* name, int rtld
     DL_ERR("file offset for the library \"%s\" is not page-aligned: %" PRId64, name, file_offset);
     return nullptr;
   }
+  if (file_offset < 0) {
+    DL_ERR("file offset for the library \"%s\" is negative: %" PRId64, name, file_offset);
+    return nullptr;
+  }
 
   struct stat file_stat;
   if (TEMP_FAILURE_RETRY(fstat(fd, &file_stat)) != 0) {
     DL_ERR("unable to stat file for the library \"%s\": %s", name, strerror(errno));
     return nullptr;
   }
+  if (file_offset >= file_stat.st_size) {
+    DL_ERR("file offset for the library \"%s\" >= file size: %" PRId64 " >= %" PRId64, name, file_offset, file_stat.st_size);
+    return nullptr;
+  }
 
   // Check for symlink and other situations where
   // file can have different names.
diff --git a/tests/dlext_test.cpp b/tests/dlext_test.cpp
index 58add6b95..5b3136458 100644
--- a/tests/dlext_test.cpp
+++ b/tests/dlext_test.cpp
@@ -17,8 +17,10 @@
 #include <gtest/gtest.h>
 
 #include <dlfcn.h>
+#include <elf.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
@@ -39,6 +41,9 @@
 #define ASSERT_NOERROR(i) \
     ASSERT_NE(-1, i) << "errno: " << strerror(errno)
 
+#define ASSERT_SUBSTR(needle, haystack) \
+    ASSERT_PRED_FORMAT2(::testing::IsSubstring, needle, haystack)
+
 
 typedef int (*fn)(void);
 #define LIBNAME "libdlext_test.so"
@@ -138,7 +143,7 @@ TEST_F(DlExtTest, ExtInfoUseFdWithInvalidOffset) {
   ASSERT_TRUE(android_data != nullptr);
 
   char lib_path[PATH_MAX];
-  snprintf(lib_path, sizeof(lib_path), LIBZIPPATH, android_data);
+  snprintf(lib_path, sizeof(lib_path), LIBPATH, android_data);
 
   android_dlextinfo extinfo;
   extinfo.flags = ANDROID_DLEXT_USE_LIBRARY_FD | ANDROID_DLEXT_USE_LIBRARY_FD_OFFSET;
@@ -149,11 +154,20 @@ TEST_F(DlExtTest, ExtInfoUseFdWithInvalidOffset) {
   ASSERT_TRUE(handle_ == nullptr);
   ASSERT_STREQ("dlopen failed: file offset for the library \"libname_placeholder\" is not page-aligned: 17", dlerror());
 
-  extinfo.library_fd_offset = (5LL<<58) + PAGE_SIZE;
+  // Test an address above 2^44, for http://b/18178121 .
+  extinfo.library_fd_offset = (5LL<<48) + PAGE_SIZE;
+  handle_ = android_dlopen_ext("libname_placeholder", RTLD_NOW, &extinfo);
+  ASSERT_TRUE(handle_ == nullptr);
+  ASSERT_SUBSTR("dlopen failed: file offset for the library \"libname_placeholder\" >= file size", dlerror());
+
+  extinfo.library_fd_offset = 0LL - PAGE_SIZE;
+  handle_ = android_dlopen_ext("libname_placeholder", RTLD_NOW, &extinfo);
+  ASSERT_TRUE(handle_ == nullptr);
+  ASSERT_SUBSTR("dlopen failed: file offset for the library \"libname_placeholder\" is negative", dlerror());
+
+  extinfo.library_fd_offset = PAGE_SIZE;
   handle_ = android_dlopen_ext("libname_placeholder", RTLD_NOW, &extinfo);
-
   ASSERT_TRUE(handle_ == nullptr);
-  // TODO: Better error message when reading with offset > file_size
   ASSERT_STREQ("dlopen failed: \"libname_placeholder\" has bad ELF magic", dlerror());
 
   close(extinfo.library_fd);