Closed Bug 740318 Opened 10 years ago Closed 9 years ago

Out of range read in Android custom dynamic linker

Categories

(Core :: mozglue, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 810478

People

(Reporter: jseward, Unassigned)

Details

(Keywords: valgrind)

Attachments

(1 file)

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)
Attached patch A possible fixSplinter Review
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+
Keywords: valgrind
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.