Heap read overflows in get_cell_size()

[Jorgecmartins]

In get_cell_size(), in ps-pdf.cxx, there are two similar heap read overflows:

9059  if ((var = htmlGetVariable(t, (uchar *)"WIDTH")) != NULL &&
9060      (var[strlen((char *)var) - 1] != '%' || (right - left) > 0.0f))
9061  {
9062    // Yes, use it!
9063    if (var[strlen((char *)var) - 1] == '%')
...

9080  // Then the height...
9081  if ((var = htmlGetVariable(t, (uchar *)"HEIGHT")) != NULL)
9082  {
9083    // Yes, use it!
9084    if (var[strlen((char *)var) - 1] == '%')
...

The vulnerabilities are triggered when strlen((char *)var) is equal to 0 and we perform var[strlen((char *)var) - 1].

The inputs that trigger the vulnerable code are easy to understand:

<TABLE>
    <TD WIDTH=""></TD>
</TABLE>

<TABLE>
    <TD HEIGHT=""></TD>
</TABLE>

I've attached poc.zip that can trigger the vulnerabilities.

Steps to reproduce

$ unzip poc.zip
$ # Compiling htmldoc with -fsanitize=address
$ htmldoc --webpage -f output.pdf height.html

=================================================================
==52227==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000004d6f at pc 0x55c9159cbb8b bp 0x7ffe4ea9cb30 sp 0x7ffe4ea9cb20
READ of size 1 at 0x602000004d6f thread T0
    #0 0x55c9159cbb8a in get_cell_size /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:9081
    #1 0x55c915a1d772 in parse_table /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:6616
    #2 0x55c915a05028 in parse_doc /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:4178
    #3 0x55c915a01939 in parse_doc /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:4524
    #4 0x55c915a5ec4b in pspdf_export /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:803
    #5 0x55c915948625 in main /home/fuzz/fuzzing/htmldoc/htmldoc/htmldoc.cxx:1291
    #6 0x7f36197e50b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #7 0x55c915957ced in _start (/home/fuzz/fuzzing/htmldoc/fuzzing/analysis/heap_overflow_get_cell_size/htmldoc_asan+0x6bced)

0x602000004d6f is located 1 bytes to the left of 1-byte region [0x602000004d70,0x602000004d71)
allocated by thread T0 here:
    #0 0x7f361a7b13dd in strdup (/lib/x86_64-linux-gnu/libasan.so.5+0x963dd)
    #1 0x55c915aa0890 in htmlSetVariable /home/fuzz/fuzzing/htmldoc/htmldoc/htmllib.cxx:2415

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:9081 in get_cell_size

$ # htmldoc compiled with -fsanitize=address
$ htmldoc --webpage -f output.pdf width.html
=================================================================
==52319==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000004d6f at pc 0x55f393d2ab8b bp 0x7ffd12a73c60 sp 0x7ffd12a73c50
READ of size 1 at 0x602000004d6f thread T0
    #0 0x55f393d2ab8a in get_cell_size /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:9081
    #1 0x55f393d7c772 in parse_table /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:6616
    #2 0x55f393d64028 in parse_doc /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:4178
    #3 0x55f393d60939 in parse_doc /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:4524
    #4 0x55f393dbdc4b in pspdf_export /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:803
    #5 0x55f393ca7625 in main /home/fuzz/fuzzing/htmldoc/htmldoc/htmldoc.cxx:1291
    #6 0x7fa50d3a00b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #7 0x55f393cb6ced in _start (/home/fuzz/fuzzing/htmldoc/fuzzing/analysis/heap_overflow_get_cell_size/htmldoc_asan+0x6bced)

0x602000004d6f is located 1 bytes to the left of 1-byte region [0x602000004d70,0x602000004d71)
allocated by thread T0 here:
    #0 0x7fa50e36c3dd in strdup (/lib/x86_64-linux-gnu/libasan.so.5+0x963dd)
    #1 0x55f393dff890 in htmlSetVariable /home/fuzz/fuzzing/htmldoc/htmldoc/htmllib.cxx:2415

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/fuzz/fuzzing/htmldoc/htmldoc/ps-pdf.cxx:9081 in get_cell_size

Looking at the source code of ps-pdf.cxx, there appears to exist similar code patterns.
Getting a variable from htmlGetVariable() and not checking if its size is 0, for instance:

5959    if (height_var[strlen((char *)height_var) - 1] == '%')

6400    if (var[strlen((char *)var) - 1] == '%')

6413    if (var[strlen((char *)var) - 1] == '%')

6619    if (var[strlen((char *)var) - 1] == '%')

6729    if (var[strlen((char *)var) - 1] == '%')

7076    if (height_var[strlen((char *)height_var) - 1] == '%')

9391    (var[strlen((char *)var) - 1] != '%'

9394    if (var[strlen((char *)var) - 1] == '%')

9409    if (var[strlen((char *)var) - 1] == '%')

9694    if (width[strlen((char *)width) - 1] == '%')

9699    if (height[strlen((char *)height) - 1] == '%')

9714    if (width[strlen((char *)width) - 1] == '%')

9723    if (height[strlen((char *)height) - 1] == '%')

[michaelrsweet]

[master 14f0d31] Fix potential heap underflow with empty attributes (Issue #464)