Closed Bug 740318 Opened 10 years ago Closed 9 years ago
Out of range read in Android custom dynamic linker
m-c, Android on Xoom, Debug intent startup with export MOZ_LINKER_EXTRACT=1 gives the trace below. (Not sure if debug intent start or the env var setting are actually necessary, but anyway). Thread 13: Invalid read of size 1 at 0x48064F8: strlen (mc_replace_strmem.c:391) by 0x4831AAD: strndup (strndup.c:33) by 0x22246B0B: report_mapping (APKOpen.cpp:681) by 0x2224B103: CustomElf::Load(Mappable*, char const*, int) (CustomElf.cpp:211) by 0x22249DC3: ElfLoader::Load(char const*, int, LibHandle*) (ElfLoader.cpp:251) by 0x22249E8B: __wrap_dlopen (ElfLoader.cpp:46) by 0x22246DF3: loadSQLiteLibs(char const*) (APKOpen.cpp:810) by 0x22246EBD: Java_org_mozilla_gecko_GeckoAppShell_loadSQLiteLibsNative (APKOpen.cpp:945) by 0x540CBF3: dvmPlatformInvoke (CallEABI.S:258) Address 0x1e345692 is 0 bytes after a block of size 658 alloc'd at 0x4805318: malloc (vg_replace_malloc.c:263) by 0x2224646D: extractBuf(char const*, Zip*) (APKOpen.cpp:648) by 0x22246DC9: loadSQLiteLibs(char const*) (APKOpen.cpp:804) by 0x22246EBD: Java_org_mozilla_gecko_GeckoAppShell_loadSQLiteLibsNative (APKOpen.cpp:945) by 0x540CBF3: dvmPlatformInvoke (CallEABI.S:258)
This stops V complaining, by allocating the relevant buffer one byte larger and zero terminating it. I'm not sure I understood the context properly though .. seems like the user of this buffer later does strlen() on it (triggering the original error), but the buffer is may be filled in by decompressing stuff via extractLib?
Attachment #612125 - Flags: review?(mh+mozilla)
Comment on attachment 612125 [details] [diff] [review] A possible fix Review of attachment 612125 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozglue/android/APKOpen.cpp @@ +645,4 @@ > if (!zip->GetStream(path, &s)) > return NULL; > > + size_t nBuf = 1 + s.GetUncompressedSize(); I'd just use s.GetUncompressedSize(), malloc nBuf + 1, and use nBuf later. And use a more explicit variable name, like size or length. Other than these nits, this is the right fix. And your understanding of the context is right.
Attachment #612125 - Flags: review?(mh+mozilla) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 810478
You need to log in before you can comment on or make changes to this bug.