stack-overflow in HTMLlineproc0

[kcwu]

input (xxd cases/tats-w3m-198)

00000000: 3c64 743e 3c2f 6464 3e30 3030 3030 3030  <dt></dd>0000000
00000010: 3030 3030 3030 3030 3030 3030 3030 3030  0000000000000000
00000020: 3030 3030 3030 3030 3030 3030 3030 3030  0000000000000000
00000030: 3030 3030 3030 3030 3030 3030 3030 3030  0000000000000000
00000040: 3030 3030 3030 3030 3030 3030 3030 3030  0000000000000000
00000050: 3030 3030 3030 3030 30                   000000000

how to reproduce:

ASAN_OPTIONS=abort_on_error=1:detect_leaks=0 ./w3m-tats.asan -T text/html -dump cases/tats-w3m-198

stderr:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==4127742==ERROR: AddressSanitizer: stack-overflow on address 0x7fff22e35b08 (pc 0x000000433153 bp 0x7fff22e36350 sp 0x7fff22e35b10 T0)
    #0 0x433153 in strlen (/w3m-tats.asan+0x433153)
    #1 0x6c9b1f in Strnew_charp /fuzz/fuzzing-w3m/targets/w3m-tats/Str.c:81:9
    #2 0x535ab3 in flushline /fuzz/fuzzing-w3m/targets/w3m-tats/file.c:2838:9
    #3 0x56ed10 in HTMLlineproc0 /fuzz/fuzzing-w3m/targets/w3m-tats/file.c:6758:7
    #4 0x56ed59 in HTMLlineproc0 /fuzz/fuzzing-w3m/targets/w3m-tats/file.c:6762:7
(skip)

SUMMARY: AddressSanitizer: stack-overflow (/w3m-tats.asan+0x433153) in strlen
==4127742==ABORTING

More detail to reproduce please see http://github.com/kcwu/fuzzing-w3m

For your convenience,
gdbline:
ASAN_OPTIONS=abort_on_error=1:detect_leaks=0 gdb --args ./w3m-tats.asan -T text/html -dump cases/tats-w3m-198

Found by AFL++

[rkta]

On Sat, Oct 09, 2021 at 09:39:21PM -0700, Kuang-che Wu wrote: input (`xxd cases/tats-w3m-198`) ``` 00000000: 3c64 743e 3c2f 6464 3e30 3030 3030 3030 <dt></dd>0000000 00000010: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000 00000020: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000 00000030: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000 00000040: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000 00000050: 3030 3030 3030 3030 30 000000000 ``` how to reproduce: ``` ASAN_OPTIONS=abort_on_error=1:detect_leaks=0 ./w3m-tats.asan -T text/html -dump cases/tats-w3m-198 ```

The input contains '0' 80 times. '-dump' sets COLS to 80. This is important, if one removes a zero the overflow does not happen (because 'need_flushline()' does not return 1). When parsing the '<dt>' (file.c:4768) it will 'PUSH_ENV_NOINDENT(HTML_DL);'. Then when parsing the '</dd>' (file.c:4820) it will check for 'envs[h_env->envc].env != HTML_DL' to return early. Since 'HTML_DL' was set before it will proceed to decrement 'indent' to -4. Now in 'HTMLlineproc0()' the condition 'if (obuf->bp.pos - i > indent)' (file.c:6747) is always true. Because of that 'HTMLlineproc1()' (file.c:6762) is called, which is just a macro for 'HTMLlineproc0()' with one parameter set to a default. From there on it will recursively call 'HTMLlineproc0()' till the kernel stops us.

[kcwu]

This issue is introduced by 77ecf9b

[bptato]

Honestly, I'm not sure what I was doing there. I know I was trying to avoid description titles being on the wrong indentation level, but using the previous one already achieved that... or at least now it does, I might have messed up a few things at first which made the NOINDENT macro necessary. Anyway, currently the only thing the macro achieves is as you described breaking </dd> when called after <dl> or <dt>.

Long story short, replacing every instance of PUSH_ENV_NOINDENT with PUSH_ENV seems to fix the issue.