out of bounds heap read in rpmstrPoolId / rstrlenhash #135

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

Comments

Projects
None yet
2 participants
@hannob

hannob commented Jan 28, 2017

The attached file will cause an out of bounds memory read in rpm (tested with rpm -i --test [input]).

rpm-oob-heap-read-rstrlenhash-rpmstrPoolId.zip

Found with american fuzzy lop and address sanitizer.

Here's a stack trace from asan:

==29668==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000151b at pc 0x0000006a0e05 bp 0x7ffe13842070 sp 0x7ffe13842068
READ of size 1 at 0x60200000151b thread T0
    #0 0x6a0e04 in rstrlenhash /f/rpm/rpm/rpmio/rpmstrpool.c:52:12
    #1 0x6a0e04 in rpmstrPoolId /f/rpm/rpm/rpmio/rpmstrpool.c:390
    #2 0x536103 in singleDS /f/rpm/rpm/lib/rpmds.c:460:15
    #3 0x536103 in rpmdsSinglePool /f/rpm/rpm/lib/rpmds.c:486
    #4 0x512720 in findPos /f/rpm/rpm/lib/depends.c:328:20
    #5 0x512720 in addPackage /f/rpm/rpm/lib/depends.c:446
    #6 0x5122e9 in rpmtsAddInstallElement /f/rpm/rpm/lib/depends.c:493:12
    #7 0x57a1d4 in rpmInstall /f/rpm/rpm/lib/rpminstall.c:584:11
    #8 0x5057ae in main /f/rpm/rpm/rpmqv.c:295:12
    #9 0x7f09b5fdc78f in __libc_start_main (/lib64/libc.so.6+0x2078f)
    #10 0x41c648 in _start (/r/rpm/rpm+0x41c648)

0x60200000151b is located 5 bytes to the right of 6-byte region [0x602000001510,0x602000001516)
allocated by thread T0 here:
    #0 0x4cc7a8 in malloc (/r/rpm/rpm+0x4cc7a8)
    #1 0x67546e in rstrdup /f/rpm/rpm/rpmio/rpmmalloc.c:74:29
    #2 0x62018f in rpmHeaderFormatCall /f/rpm/rpm/lib/formats.c:541:8
    #3 0x612486 in rpmtdFormat /f/rpm/rpm/lib/rpmtd.c:261:8
    #4 0x58f207 in addTE /f/rpm/rpm/lib/rpmte.c:145:15
    #5 0x58f207 in rpmteNew /f/rpm/rpm/lib/rpmte.c:241
    #6 0x512642 in addPackage /f/rpm/rpm/lib/depends.c:438:9
@hannob

This comment has been minimized.

Show comment
Hide comment
@hannob

hannob Jan 28, 2017

I'm attaching another file, this creates a use after free, but it's in the same line of code, so I assume it's a variation of the same bug.
rpm-useafterfree-rstrlenhash-rpmstrPoolId.zip

==26753==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000001531 at pc 0x0000006a0e05 bp 0x7ffc05f97c30 sp 0x7ffc05f97c28
READ of size 1 at 0x602000001531 thread T0
    #0 0x6a0e04 in rstrlenhash /f/rpm/rpm/rpmio/rpmstrpool.c:52:12
    #1 0x6a0e04 in rpmstrPoolId /f/rpm/rpm/rpmio/rpmstrpool.c:390
    #2 0x536103 in singleDS /f/rpm/rpm/lib/rpmds.c:460:15
    #3 0x536103 in rpmdsSinglePool /f/rpm/rpm/lib/rpmds.c:486
    #4 0x512720 in findPos /f/rpm/rpm/lib/depends.c:328:20
    #5 0x512720 in addPackage /f/rpm/rpm/lib/depends.c:446
    #6 0x5122e9 in rpmtsAddInstallElement /f/rpm/rpm/lib/depends.c:493:12
    #7 0x57a1d4 in rpmInstall /f/rpm/rpm/lib/rpminstall.c:584:11
    #8 0x5057ae in main /f/rpm/rpm/rpmqv.c:295:12
    #9 0x7f4f83a7678f in __libc_start_main (/lib64/libc.so.6+0x2078f)
    #10 0x41c648 in _start (/r/rpm/rpm+0x41c648)

0x602000001531 is located 1 bytes inside of 6-byte region [0x602000001530,0x602000001536)
freed by thread T0 here:
    #0 0x4cc5f0 in __interceptor_cfree.localalias.1 (/r/rpm/rpm+0x4cc5f0)
    #1 0x60ff7f in rpmtdFreeData /f/rpm/rpm/lib/rpmtd.c:48:2
    #2 0x58f207 in addTE /f/rpm/rpm/lib/rpmte.c:145:15
    #3 0x58f207 in rpmteNew /f/rpm/rpm/lib/rpmte.c:241
    #4 0x512642 in addPackage /f/rpm/rpm/lib/depends.c:438:9

previously allocated by thread T0 here:
    #0 0x4ccbc0 in realloc (/r/rpm/rpm+0x4ccbc0)
    #1 0x6752ea in rrealloc /f/rpm/rpm/rpmio/rpmmalloc.c:65:13
    #2 0x629bb4 in getNEVRA /f/rpm/rpm/lib/tagexts.c:772:11
    #3 0x625026 in nevrTag /f/rpm/rpm/lib/tagexts.c:805:12

hannob commented Jan 28, 2017

I'm attaching another file, this creates a use after free, but it's in the same line of code, so I assume it's a variation of the same bug.
rpm-useafterfree-rstrlenhash-rpmstrPoolId.zip

==26753==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000001531 at pc 0x0000006a0e05 bp 0x7ffc05f97c30 sp 0x7ffc05f97c28
READ of size 1 at 0x602000001531 thread T0
    #0 0x6a0e04 in rstrlenhash /f/rpm/rpm/rpmio/rpmstrpool.c:52:12
    #1 0x6a0e04 in rpmstrPoolId /f/rpm/rpm/rpmio/rpmstrpool.c:390
    #2 0x536103 in singleDS /f/rpm/rpm/lib/rpmds.c:460:15
    #3 0x536103 in rpmdsSinglePool /f/rpm/rpm/lib/rpmds.c:486
    #4 0x512720 in findPos /f/rpm/rpm/lib/depends.c:328:20
    #5 0x512720 in addPackage /f/rpm/rpm/lib/depends.c:446
    #6 0x5122e9 in rpmtsAddInstallElement /f/rpm/rpm/lib/depends.c:493:12
    #7 0x57a1d4 in rpmInstall /f/rpm/rpm/lib/rpminstall.c:584:11
    #8 0x5057ae in main /f/rpm/rpm/rpmqv.c:295:12
    #9 0x7f4f83a7678f in __libc_start_main (/lib64/libc.so.6+0x2078f)
    #10 0x41c648 in _start (/r/rpm/rpm+0x41c648)

0x602000001531 is located 1 bytes inside of 6-byte region [0x602000001530,0x602000001536)
freed by thread T0 here:
    #0 0x4cc5f0 in __interceptor_cfree.localalias.1 (/r/rpm/rpm+0x4cc5f0)
    #1 0x60ff7f in rpmtdFreeData /f/rpm/rpm/lib/rpmtd.c:48:2
    #2 0x58f207 in addTE /f/rpm/rpm/lib/rpmte.c:145:15
    #3 0x58f207 in rpmteNew /f/rpm/rpm/lib/rpmte.c:241
    #4 0x512642 in addPackage /f/rpm/rpm/lib/depends.c:438:9

previously allocated by thread T0 here:
    #0 0x4ccbc0 in realloc (/r/rpm/rpm+0x4ccbc0)
    #1 0x6752ea in rrealloc /f/rpm/rpm/rpmio/rpmmalloc.c:65:13
    #2 0x629bb4 in getNEVRA /f/rpm/rpm/lib/tagexts.c:772:11
    #3 0x625026 in nevrTag /f/rpm/rpm/lib/tagexts.c:805:12
@pmatilai

This comment has been minimized.

Show comment
Hide comment
@pmatilai

pmatilai Jan 31, 2017

Contributor

Thanks for the pile of reports, will start looking into them once recovered from devconf.cz trip.

Contributor

pmatilai commented Jan 31, 2017

Thanks for the pile of reports, will start looking into them once recovered from devconf.cz trip.

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 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