Out of range read in Android custom dynamic linker

RESOLVED DUPLICATE of bug 810478

Status

()

RESOLVED DUPLICATE of bug 810478
7 years ago
6 years ago

People

(Reporter: jseward, Unassigned)

Tracking

({valgrind})

Trunk
x86_64
Linux
valgrind
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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)
(Reporter)

Comment 1

7 years ago
Created attachment 612125 [details] [diff] [review]
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?
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+
(Reporter)

Updated

6 years ago
Keywords: valgrind
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 810478
You need to log in before you can comment on or make changes to this bug.