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

Integer-overflow in function getElfSections at p_vmlinx.cpp:119 #286

Closed
aheroine opened this issue Jul 19, 2019 · 7 comments
Closed

Integer-overflow in function getElfSections at p_vmlinx.cpp:119 #286

aheroine opened this issue Jul 19, 2019 · 7 comments
Labels

Comments

@aheroine
Copy link

aheroine commented Jul 19, 2019

What's the problem (or question)?

upx.out -l @@ /dev/null

An issue was discovered in upx 3.95, There is a/an Integer-overflow in function
getElfSections at p_vmlinx.cpp:119

In the add operation, the result of (p->sh_size + p->sh_offset) is equal to zero because of overflow. 
So the execution can pass the condition check on line 114 and applicate excessive memory line 119, 
resulting in denial of service.

What should have happened?

list compressed file

Do you have an idea for a solution?

check for integer overflow correctly

How can we reproduce the issue?

upx.out -l @@ /dev/null

the poc is attached poc

source

file: p_vmlinx.cpp
function: getElfSections() 
 104 template <class T>
 105 typename T::Shdr const *PackVmlinuxBase<T>::getElfSections()
 106 {
 107     Shdr const *p, *shstrsec=0;
 108     shdri = new Shdr[(unsigned) ehdri.e_shnum];
 109     fi->seek(ehdri.e_shoff, SEEK_SET);
 110     fi->readx(shdri, ehdri.e_shnum * sizeof(*shdri));
 111     int j;
 112     for (p = shdri, j= ehdri.e_shnum; --j>=0; ++p) {
 113         if (Shdr::SHT_STRTAB==p->sh_type
 114         &&  (p->sh_size + p->sh_offset) <= (unsigned long)file_size
 115         &&       p->sh_name  <  p->sh_size
 116         &&  (10+ p->sh_name) <= p->sh_size  // 1+ strlen(".shstrtab")
 117         ) {
 118             delete [] shstrtab;
 119             shstrtab = new char[1+ p->sh_size];
 120             fi->seek(p->sh_offset, SEEK_SET);
 121             fi->readx(shstrtab, p->sh_size);
 122             shstrtab[p->sh_size] = '\0';
 123             if (0==strcmp(".shstrtab", shstrtab + p->sh_name)) {
 124                 shstrsec = p;
 125                 break;
 126             }
 127         }
 128     }
 129     return shstrsec;
 130 }

debug

(gdb) p (unsigned long long)p->sh_size + (unsigned long long)p->sh_offset
$4 = 0
(gdb) p (unsigned long long)p->sh_size
$5 = 576460752303423488
(gdb) p  (unsigned long long)p->sh_offset
$6 = 17870283321406128128
(gdb) p p->sh_size
$7 = {d = "\000\000\000\000\000\000\000\b"}
(gdb) p p->sh_offset 
$8 = {d = "\000\000\000\000\000\000", <incomplete sequence \370>}
(gdb) x/gx &p->sh_size 
0x61c00000ff20:	0x0800000000000000
(gdb) x/gx &p->sh_offset 
0x61c00000ff18:	0xf800000000000000

bug report

=================================================================
==6768==WARNING: AddressSanitizer failed to allocate 0x800000000000001 bytes
==6768==AddressSanitizer's allocator is terminating the process instead of returning 0
==6768==If you don't like this behavior set allocator_may_return_null=1
==6768==AddressSanitizer CHECK failed: ../../../../src/libsanitizer/sanitizer_common/sanitizer_allocator.cc:147 "((0)) != (0)" (0x0, 0x0)
    #0 0x7f935b1a2631  (/usr/lib/x86_64-linux-gnu/libasan.so.2+0xa0631)
    #1 0x7f935b1a75e3 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0xa55e3)
    #2 0x7f935b11f425  (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x1d425)
    #3 0x7f935b1a5865  (/usr/lib/x86_64-linux-gnu/libasan.so.2+0xa3865)
    #4 0x7f935b124b4d  (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x22b4d)
    #5 0x7f935b19b67e in operator new[](unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9967e)
    #6 0x6f73e2 in PackVmlinuxBase<N_Elf::ElfClass_64<N_BELE_CTP::LEPolicy> >::getElfSections() /home/jl/work/projects/upx/upx/src/p_vmlinx.cpp:119
    #7 0x70949a in PackVmlinuxBase<N_Elf::ElfClass_64<N_BELE_CTP::LEPolicy> >::canUnpack() /home/jl/work/projects/upx/upx/src/p_vmlinx.cpp:569
    #8 0x7a7e1d in try_unpack /home/jl/work/projects/upx/upx/src/packmast.cpp:114
    #9 0x7abe70 in PackMaster::visitAllPackers(Packer* (*)(Packer*, void*), InputFile*, options_t const*, void*) /home/jl/work/projects/upx/upx/src/packmast.cpp:177
    #10 0x7b5282 in PackMaster::getUnpacker(InputFile*) /home/jl/work/projects/upx/upx/src/packmast.cpp:248
    #11 0x7b694b in PackMaster::list() /home/jl/work/projects/upx/upx/src/packmast.cpp:279
    #12 0x8604db in do_one_file(char const*, char*) /home/jl/work/projects/upx/upx/src/work.cpp:164
    #13 0x8615fa in do_files(int, int, char**) /home/jl/work/projects/upx/upx/src/work.cpp:271
    #14 0x468b28 in main /home/jl/work/projects/upx/upx/src/main.cpp:1539
    #15 0x7f935989982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #16 0x4030f8 in _start (/home/jl/work/up_projects/fuzz-upx/crashes/upx.out.debug+0x4030f8)

Please tell us details about your environment.

  • UPX version used (upx --3.95):
  • Host Operating System and version: ubuntu 16.04
  • Host CPU architecture: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
  • Target Operating System and version: ubuntu 16.04
  • Target CPU architecture: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
@jreiser
Copy link
Collaborator

jreiser commented Jul 19, 2019

Correctly diagnosed by tip of devel branch:

$ upx.out -f -o foo poc-Integer-overflow
                       Ultimate Packer for eXecutables
                          Copyright (C) 1996 - 2019
UPX git-593a69+ Markus Oberhumer, Laszlo Molnar & John Reiser   Feb 24th 2019

        File size         Ratio      Format      Name
   --------------------   ------   -----------   -----------
upx.out: poc-Integer-overflow: CantPackException: bad e_phoff

Packed 0 files.

WARNING: this is an unstable beta version - use for testing only! Really.

jreiser added a commit that referenced this issue Jul 21, 2019
@jreiser
Copy link
Collaborator

jreiser commented Jul 21, 2019

A better fix on devel branch via commit above.

@limburgher
Copy link

Is a release forthcoming to address this and #287, or should packagers ship patches?

@jreiser
Copy link
Collaborator

jreiser commented Aug 1, 2019

Only Markus can make an official release of UPX, and Markus is busy. History suggests that it would take at least several weeks. Only Markus has the source for the NRV compression algorithm that is used in official releases. The UCL algorithm used in devel is open source and compatible for de-compression; but output generated by NRV usually is just slightly smaller. I suggest that packagers ship patches.

@limburgher
Copy link

Will do, thank you.

@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 15, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please feel free to reopen.

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

No branches or pull requests

3 participants