Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double free ( memory clobbered ) in PackLinuxElf64::unpack #207

Closed
math1as opened this issue May 18, 2018 · 9 comments
Closed

Double free ( memory clobbered ) in PackLinuxElf64::unpack #207

math1as opened this issue May 18, 2018 · 9 comments
Assignees
Milestone

Comments

@math1as
Copy link

math1as commented May 18, 2018

./upx.out --version
upx 3.95-git-d9288213ec15

this is a bug in decompression (upx -d or upx -t) , it's more significant that bugs in compression.
crash when try to free an invalid pointer , the mem.cpp detect invalid double free and throw an exception (SIGABRT)

dddd

#9 0x0000000000465a65 in throwInternalError (msg=msg@entry=0x67a348 "memory clobbered past end of allocated block") at except.cpp:155
#10 0x00000000004dacce in MemBuffer::checkState (this=0x7fffffffd360) at mem.cpp:180
#11 MemBuffer::dealloc (this=0x7fffffffd360) at mem.cpp:92
#12 MemBuffer::~MemBuffer (this=0x7fffffffd360, __in_chrg=) at mem.cpp:73
#13 0x00000000005045bc in PackLinuxElf64::unpack (this=, fo=) at p_lx_elf.cpp:3923

when an attacker could make a fake chunk in that pointer , it caused a double-free vulnerability
and attacker is able to make a code execution.

@math1as
Copy link
Author

math1as commented May 18, 2018

poc here
poc_free.zip

@math1as math1as changed the title free invalid pointer in PackLinuxElf64::unpack Double free ( memory clobbered ) in PackLinuxElf64::unpack May 18, 2018
@roblub
Copy link

roblub commented Aug 5, 2018

It looks like the behavior was changed in commit 9fddd: work in progress: de-compression of --android-shlib . Before the commit upx -d /tmp/poc_free returned an error:

 UPX git-7d2913  Markus Oberhumer, Laszlo Molnar & John Reiser   May 13th 2017

        File size         Ratio      Format      Name
   --------------------   ------   -----------   -----------
 upx.out: /tmp/poc_free: Exception: compressed data violation

 Unpacked 1 file: 0 ok, 1 error.

And after the commit, the above upx command crashes:

 UPX git-9fdddf  Markus Oberhumer, Laszlo Molnar & John Reiser   May 13th 2017

        File size         Ratio      Format      Name
   --------------------   ------   -----------   -----------
 terminate called after throwing an instance of '13InternalError'
   what():  std::exception
 Aborted (core dumped)

@jreiser
Copy link
Collaborator

jreiser commented Aug 25, 2018

Markus, this is a bug in ucl-1.03. I have a patch, but the ucl-1.03 source has bitrot and cannot be compiled.

The fix that I want to try for ucl-1.03 is:

$ diff -Nur ./src/n2b_d.c.orig ./src/n2b_d.c
--- ./src/n2b_d.c.orig    2004-07-19 16:01:47.000000000 -0700
+++ ./src/n2b_d.c    2018-05-18 19:20:24.426904865 -0700
@@ -101,7 +101,7 @@
             m_len += 2;
         }
         m_len += (m_off > 0xd00);
-        fail(olen + m_len > oend, UCL_E_OUTPUT_OVERRUN);
+        fail(olen + (m_len + 1) > oend, UCL_E_OUTPUT_OVERRUN);
         fail(m_off > olen, UCL_E_LOOKBEHIND_OVERRUN);
 #ifdef TEST_OVERLAP
         olen += m_len + 1;
$ diff -Nur ./src/n2e_d.c.orig ./src/n2e_d.c
--- ./src/n2e_d.c.orig    2004-07-19 16:01:47.000000000 -0700
+++ ./src/n2e_d.c    2018-05-18 19:21:54.698069323 -0700
@@ -109,7 +109,7 @@
             m_len += 3;
         }
         m_len += (m_off > 0x500);
-        fail(olen + m_len > oend, UCL_E_OUTPUT_OVERRUN);
+        fail(olen + (m_len + 1) > oend, UCL_E_OUTPUT_OVERRUN);
         fail(m_off > olen, UCL_E_LOOKBEHIND_OVERRUN);
 #ifdef TEST_OVERLAP
         olen += m_len + 1;

I believe that the "m_len + 1" in

+        fail(olen + (m_len + 1) > oend, UCL_E_OUTPUT_OVERRUN);

should match the "m_len + 1" in

         olen += m_len + 1;

because it is the number of increments of olen in the copy step:

            dst[olen++] = *m_pos++;
            do dst[olen++] = *m_pos++; while (--m_len > 0);

Trying to re-build ucl-1.03 fails because: (./config.log)

configure:2064: gcc -V </dev/null >&5
gcc: error: unrecognized command line option '-V'
gcc: fatal error: no input files
compilation terminated.

@roblub
Copy link

roblub commented Aug 27, 2018

John,

To compile ucl-1.03 you might want to apply 04-Static-assert.patch from https://sources.debian.org/src/ucl/1.03+repack-4/debian/patches/

Anyway I've just checked that after patching n2b_d.c and n2e_d.c files in ucl, upx returns the "Exception: compressed data violation" as it used to before the commit 9fdddf.

@jreiser
Copy link
Collaborator

jreiser commented Sep 3, 2018

@roblub Yes, the 04-Static-assert.patch fixes the compile error during configuration. Thank you. Also, the complaint from upx "Exception: compressed data violation" is correct; the input file poc_free is from a fuzzer seeking to test the robustness of upx -d against malformed input.

@markus-oberhumer The patch in #207 (comment) does fix the bug in ucl-1.03. The patches from roblub #207 (comment) also should be applied to update UCL to a more modern software environment.

@markus-oberhumer markus-oberhumer modified the milestones: v3.96, v3.97 Jan 8, 2020
@NicoleG25
Copy link

Was this issue ever addressed?
Can't figure out if d928821 refers to CVE-2018-11243 or this does..

Thanks in advance !

@jengelh
Copy link

jengelh commented Jan 23, 2020

Was this issue ever addressed?

The CVE references two Github tickets (and so perhaps should have been two CVEs); only #206 applies to upx, #207 is ucl (I find no n2b_d.c in the upx repo).

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Aug 18, 2022
@markus-oberhumer
Copy link
Collaborator

Fixed in devel4 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants