michaelrsweet / htmldoc

HTML Conversion Software
https://www.msweet.org/htmldoc
GNU General Public License v2.0
208 stars 47 forks source link

Out of bounds read in function #433

Closed chibataiki closed 3 years ago

chibataiki commented 3 years ago

Hi ,

In function parse_tree() in toc.cxx, there is a out-of-bounds read bug.

If the value of heading_numbers[i] equal to 1000, then the heading_numbers[i] / 100 = 10 , but the length of array HUNDREDS and hundreds is 10, so out of bounds read occurred.

232               case 'i' :
233                   snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%s%s%s", hundreds[heading_numbers[i] / 100],     tens[(heading_numbers[i] / 10) % 10], ones[heading_numbers[i] % 10]);
234                   break;
235               case 'I' :
236                   snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%s%s%s", HUNDREDS[heading_numbers[i] / 100],     TENS[(heading_numbers[i] / 10) % 10], ONES[heading_numbers[i] % 10]);
237                   break;

Version: 1.9.12 commit [a5b87b9] Env: ubuntu 20.04 x86_64 clang version 11.0.0

reproduce ./configure make ./htmldoc [poc]

oobr_parse_tree.poc.zip

more

gdb

#3  0x00007ffff7a04f76 in __GI___snprintf (s=<optimized out>, maxlen=<optimized out>, format=<optimized out>) at snprintf.c:31
#4  0x0000000000462ae9 in parse_tree (t=0xc50620) at toc.cxx:236
#5  0x0000000000463047 in parse_tree (t=0xc500a0) at toc.cxx:375
#6  0x0000000000463047 in parse_tree (t=0x99c9b0) at toc.cxx:375
#7  0x0000000000463047 in parse_tree (t=0x968080) at toc.cxx:375
#8  0x0000000000463047 in parse_tree (t=0x967900) at toc.cxx:375
#9  0x0000000000463047 in parse_tree (t=0x9401c0) at toc.cxx:375
#10 0x0000000000462439 in toc_build (tree=0x9401c0) at toc.cxx:81
#11 0x000000000040cfa7 in main (argc=0x2, argv=0x7fffffffe368) at htmldoc.cxx:1275

gef➤   p heading_numbers[i] / 100
$24 = 0xa
gef➤  p HUNDREDS[heading_numbers[i] / 100]
$25 = 0x3131313131313149 <error: Cannot access memory at address 0x3131313131313149>
==311711== Invalid read of size 1
==311711==    at 0x483EF46: strlen (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==311711==    by 0x4CFCE94: __vfprintf_internal (vfprintf-internal.c:1688)
==311711==    by 0x4D10119: __vsnprintf_internal (vsnprintf.c:114)
==311711==    by 0x4CE5F75: snprintf (snprintf.c:31)
==311711==    by 0x462AE8: parse_tree(tree_str*) (toc.cxx:236)
==311711==    by 0x463046: parse_tree(tree_str*) (toc.cxx:375)
==311711==    by 0x463046: parse_tree(tree_str*) (toc.cxx:375)
==311711==    by 0x463046: parse_tree(tree_str*) (toc.cxx:375)
==311711==    by 0x463046: parse_tree(tree_str*) (toc.cxx:375)
==311711==    by 0x463046: parse_tree(tree_str*) (toc.cxx:375)
==311711==    by 0x462438: toc_build (toc.cxx:81)
==311711==    by 0x40CFA6: main (htmldoc.cxx:1275)
==311711==  Address 0x3131313131313149 is not stack'd, malloc'd or (recently) free'd
michaelrsweet commented 3 years ago

Confirmed, will decide what to do with this (not likely in anything but a malicious file...)

michaelrsweet commented 3 years ago

[master c67bbd8] Fix array overflow for headings using roman numerals (Issue #433)

I consolidated everything in format_number, which already handled the overflow. But I also updated format_number to roll the numbers over and to support roman numerals up to 2999 before wrapping.