740318 - Out of range read in Android custom dynamic linker

Status: RESOLVED DUPLICATE of bug 810478

(Core :: mozglue, defect)

Description

[Julian Seward [:jseward]]

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)

Comment 1

[Julian Seward [:jseward]]

Attachment: A possible fix

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?

Comment 2

[Mike Hommey [:glandium]]

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.