invalid read in dataLength / grabData (header.c) #138

Closed
hannob opened this Issue Jan 28, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@hannob

hannob commented Jan 28, 2017

The attached file causes an invalid memory read access.
rpm-invalidread-dataLength-grabData.zip

asan error:

==16740==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fd8bbe403c6 bp 0x7fff586b2130 sp 0x7fff586b18b8 T0)
==16740==The signal is caused by a READ memory access.
==16740==Hint: address points to the zero page.
    #0 0x7fd8bbe403c5 in strlen (/lib64/libc.so.6+0x7e3c5)
    #1 0x43b8bc in __interceptor_strlen.part.25 (/r/rpm/rpm+0x43b8bc)
    #2 0x5dbdd8 in dataLength /f/rpm/rpm/lib/header.c:432:13
    #3 0x5dbdd8 in grabData /f/rpm/rpm/lib/header.c:1364
    #4 0x5d95bc in intAddEntry /f/rpm/rpm/lib/header.c:1390:12
    #5 0x5d8a50 in headerPut /f/rpm/rpm/lib/header.c:1463:7
    #6 0x5b5c55 in addPrefixes /f/rpm/rpm/lib/relocation.c:64:3
    #7 0x5b5c55 in rpmRelocateFileList /f/rpm/rpm/lib/relocation.c:135
    #8 0x593a2f in getFiles /f/rpm/rpm/lib/rpmte.c:106:3
    #9 0x58f5db in addTE /f/rpm/rpm/lib/rpmte.c:173:16
    #10 0x58f5db in rpmteNew /f/rpm/rpm/lib/rpmte.c:241
    #11 0x512642 in addPackage /f/rpm/rpm/lib/depends.c:438:9
    #12 0x5122e9 in rpmtsAddInstallElement /f/rpm/rpm/lib/depends.c:493:12
    #13 0x57a1d4 in rpmInstall /f/rpm/rpm/lib/rpminstall.c:584:11
    #14 0x5057ae in main /f/rpm/rpm/rpmqv.c:295:12
    #15 0x7fd8bbde278f in __libc_start_main (/lib64/libc.so.6+0x2078f)
    #16 0x41c648 in _start (/r/rpm/rpm+0x41c648)

pmatilai added a commit that referenced this issue Feb 3, 2017

Sanity check header tag values. Like, doh.
There's a check for total number of tags, and their types and all
but absolutely no check for the actual tag numbers. So we end up
accepting negative tags which should not exist. The tag type should
really be uint32_t but that's another can of worms, lets have something
easily backportable for now.

This is enough to fix issues #133, #135, #136, #138 and #139 on the
level of detecting header structural inconsistency.
@pmatilai

This comment has been minimized.

Show comment
Hide comment
@pmatilai

pmatilai Feb 7, 2017

Contributor

The package is no longer readable after commit 3a07ba3 because there's a negative tag involved. However the actual crash here is due to RPMTAG_PREFIXES type mismatch (int32 in package, assumed string array) combined with lack of validation and error checking - rpm assumes tags are of correct type almost everywhere throughout the codebase. Sigh.

Contributor

pmatilai commented Feb 7, 2017

The package is no longer readable after commit 3a07ba3 because there's a negative tag involved. However the actual crash here is due to RPMTAG_PREFIXES type mismatch (int32 in package, assumed string array) combined with lack of validation and error checking - rpm assumes tags are of correct type almost everywhere throughout the codebase. Sigh.

pmatilai added a commit that referenced this issue Feb 16, 2017

Sanity check header tag values. Like, doh.
There's a check for total number of tags, and their types and all
but absolutely no check for the actual tag numbers. So we end up
accepting negative tags which should not exist. The tag type should
really be uint32_t but that's another can of worms, lets have something
easily backportable for now.

This is enough to fix issues #133, #135, #136, #138 and #139 on the
level of detecting header structural inconsistency.

Backported from commit 3a07ba3:
headerVerifyInfo() is so different in git master we can't use the
same exact thing here. Instead we do things in two steps,
headerVerifyInfo() catches totally garbage values and duplicate
regions are caught in regionSwab().
@pmatilai

This comment has been minimized.

Show comment
Hide comment
@pmatilai

pmatilai Jun 28, 2017

Contributor

The immediate crasher was already addressed, the underlying larger issue of tag validation will be tracked in #242 from here on.

Contributor

pmatilai commented Jun 28, 2017

The immediate crasher was already addressed, the underlying larger issue of tag validation will be tracked in #242 from here on.

@pmatilai pmatilai closed this Jun 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment