search

liblouis / liblouisutdml

An open-source library providing complete braille transcription services for xml, html and text documents
http://liblouis.io
GNU General Public License v3.0
24 stars 16 forks source link

Fix buffer overflow #67

Closed sthibaul closed 4 years ago

sthibaul commented 4 years ago

"which" should can not be equal to the number of inserts.

The issue was detected by using CFLAGS=-fsanitize=address while running test_mathml_woluwe/test_089.test and test_mathml_woluwe/test_097.test:

==522849==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6070000011f0 at pc 0x7fc48ececed1 bp 0x7ffe9fc17990 sp 0x7ffe9fc17988
READ of size 4 at 0x6070000011f0 thread T0
    #0 0x7fc48ececed0 in insert_code /tmp/liblouisutdml-2.8.0/liblouisutdml/semantics.c:537
    #1 0x7fc48ed00f79 in transcribe_math /tmp/liblouisutdml-2.8.0/liblouisutdml/transcribe_math.c:87
    #2 0x7fc48ed01046 in transcribe_math /tmp/liblouisutdml-2.8.0/liblouisutdml/transcribe_math.c:92
    #3 0x7fc48ed01046 in transcribe_math /tmp/liblouisutdml-2.8.0/liblouisutdml/transcribe_math.c:92
    #4 0x7fc48ecff995 in transcribe_document /tmp/liblouisutdml-2.8.0/liblouisutdml/transcribe_document.c:88
    #5 0x7fc48ecd038e in processXmlDocument /tmp/liblouisutdml-2.8.0/liblouisutdml/liblouisutdml.c:180
    #6 0x7fc48ecd114a in lbu_translateFile /tmp/liblouisutdml-2.8.0/liblouisutdml/liblouisutdml.c:283
    #7 0x562a2234f080 in main /tmp/liblouisutdml-2.8.0/tools/file2brl.c:351
    #8 0x7fc48e059bba in __libc_start_main ../csu/libc-start.c:308
    #9 0x562a2234d3a9 in _start (/tmp/liblouisutdml-2.8.0/tools/.libs/file2brl+0x83a9)

0x6070000011f0 is located 0 bytes to the right of 80-byte region [0x6070000011a0,0x6070000011f0)
allocated by thread T0 here:
    #0 0x7fc48f0ca628 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
    #1 0x7fc48ecec694 in encodeInsertions /tmp/liblouisutdml-2.8.0/liblouisutdml/semantics.c:489
    #2 0x7fc48ecef2a9 in compileLine /tmp/liblouisutdml-2.8.0/liblouisutdml/semantics.c:785
    #3 0x7fc48ecf04e7 in sem_compileFile /tmp/liblouisutdml-2.8.0/liblouisutdml/semantics.c:901
    #4 0x7fc48ecf1249 in compile_semantic_table /tmp/liblouisutdml-2.8.0/liblouisutdml/semantics.c:967
    #5 0x7fc48ecd0348 in processXmlDocument /tmp/liblouisutdml-2.8.0/liblouisutdml/liblouisutdml.c:168
    #6 0x7fc48ecd114a in lbu_translateFile /tmp/liblouisutdml-2.8.0/liblouisutdml/liblouisutdml.c:283
    #7 0x562a2234f080 in main /tmp/liblouisutdml-2.8.0/tools/file2brl.c:351
    #8 0x7fc48e059bba in __libc_start_main ../csu/libc-start.c:308
egli commented 4 years ago

So I don't quite get it. The AddressSanitizer report says that you have a buffer overflow at line 537 where it says:

insertLength = inserts->charInserts[sumLength] - 1;

How is your patch helping to avoid this?

sthibaul commented 4 years ago

What happens here is that which is equal to inserts->numInserts. And thus the for (k) loop before that line will iterate over the whole inserts chain, before that line tryies to access insert index which, thus exactly outside the array.

which being equal to inserts->numInserts is an error anyway, so 1 should just be returned in that case, just like the existing test does for the specific case where inserts->numInserts happens to be just 1.