metamath / metamath-exe

Metamath program - source code for the Metamath executable
GNU General Public License v2.0
77 stars 25 forks source link

Fix double quote processing in readTexDefs #90

Closed jamesjer closed 2 years ago

jamesjer commented 2 years ago

After yesterday's valgrind find, I thought it might be interesting to build with -fsanitize=address. Things went well until I decided to run verify markup */file_skip/top_date_skip, which resulted in this:

Checking latexdef, htmldef, althtmldef...
=================================================================
==25325==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000566c01 at pc 0x000000501d07 bp 0x7ffea13e1d80 sp 0x7ffea13e1d78
READ of size 1 at 0x000000566c01 thread T0
    #0 0x501d06 in readTexDefs /home/jamesjer/src/forked/github/metamath-exe/src/mmwtex.c:482
    #1 0x44105e in verifyMarkup /home/jamesjer/src/forked/github/metamath-exe/src/mmcmds.c:5110
    #2 0x41f817 in command /home/jamesjer/src/forked/github/metamath-exe/src/metamath.c:6424
    #3 0x4025f8 in main /home/jamesjer/src/forked/github/metamath-exe/src/metamath.c:768
    #4 0x7fa505fe854f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #5 0x7fa505fe8608 in __libc_start_main_impl ../csu/libc-start.c:389
    #6 0x402934 in _start (/home/jamesjer/src/forked/github/metamath-exe/metamath+0x402934)

0x000000566c01 is located 63 bytes to the left of global variable '*.LC6' defined in '/home/jamesjer/src/forked/github/metamath-exe/src/mmvstr.c' (0x566c40) of size 46
  '*.LC6' is ascii string '*** FATAL ERROR ***  Too many cat() arguments'
0x000000566c01 is located 0 bytes to the right of global variable '*.LC3' defined in '/home/jamesjer/src/forked/github/metamath-exe/src/mmvstr.c' (0x566c00) of size 1
  '*.LC3' is ascii string ''
SUMMARY: AddressSanitizer: global-buffer-overflow /home/jamesjer/src/forked/github/metamath-exe/src/mmwtex.c:482 in readTexDefs
Shadow bytes around the buggy address:
  0x0000800a4d30: 00 00 00 07 f9 f9 f9 f9 00 00 00 04 f9 f9 f9 f9
  0x0000800a4d40: 00 03 f9 f9 f9 f9 f9 f9 05 f9 f9 f9 f9 f9 f9 f9
  0x0000800a4d50: 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x0000800a4d60: 00 00 00 f9 f9 f9 f9 f9 00 00 00 00 00 00 05 f9
  0x0000800a4d70: f9 f9 f9 f9 00 00 00 00 00 00 00 01 f9 f9 f9 f9
=>0x0000800a4d80:[01]f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 06 f9 f9
  0x0000800a4d90: f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0000800a4da0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0000800a4db0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0000800a4dc0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0000800a4dd0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==25325==ABORTING

In this code, we are processing something like htmldef "foo" as "first string" + "second string";. The variable token is meant to hold the result of pasting strings together. The variable partialToken holds one of the individual strings; e.g, first string. The for loop checks for double quote characters inside the string, which should be replaced with a single quote character. However, the code accesses the wrong variable, token instead of partialToken. On the first pass through this loop, token is the empty string, but we try to access indices in it that are relevant to partialToken, resulting in the error above.

With this change execution continues past the "Checking latexdef, htmldef, althtmldef..." phase, but then hits another error in the "Checking bibliographic references" phase. I will make a separate PR for that issue.