global-buffer-overflow in lou_logFile() when long filename is given

[Marsman1996]

Summary

When long filename (larger than 256) is given to API lou_logFile(), there will be a global-buffer-overflow.

liblouis/liblouis/logging.c

Lines 121 to 130 in 517f6f1

	static char initialLogFileName[256] = "";
	void EXPORT_CALL
	lou_logFile(const char *fileName) {
	if (logFile) {
	fclose(logFile);
	logFile = NULL;
	}
	if (fileName == NULL || fileName[0] == 0) return;
	if (initialLogFileName[0] == 0) strcpy(initialLogFileName, fileName);

Test Environment

Ubuntu 16.04.3 LTS
liblouis (master, 6223f21)

How to trigger

1. Compile liblouis with AddressSanitizer
2. Compile the fuzz driver and poc file
3. Compile the fuzz driver: $ clang -g -fsanitize=address,fuzzer ./driver-API-6223f21-lou_logFile-BO.c ./bin_asan/lib/liblouis.a -I ./bin_asan/include/liblouis/ -o driver-API-6223f21-lou_logFile-BO
4. run the compiled driver: $ ./driver-API-6223f21-lou_logFile-BO poc-API-6223f21-lou_logFile-BO

ASAN report

$ ./driver-API-6223f21-lou_logFile-BO poc-API-6223f21-lou_logFile-BO
Minimum size is 0
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2700932249
INFO: Loaded 1 modules   (57 inline 8-bit counters): 57 [0x7b0360, 0x7b0399), 
INFO: Loaded 1 PC tables (57 PCs): 57 [0x5705c0,0x570950), 
./driver-API-6223f21-lou_logFile-BO: Running 1 inputs 1 time(s) each.
Running: poc-API-6223f21-lou_logFile-BO
=================================================================
==20412==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00000108b6e0 at pc 0x00000050dbf8 bp 0x7ffe6a0ffac0 sp 0x7ffe6a0ff280
WRITE of size 4098 at 0x00000108b6e0 thread T0
    #0 0x50dbf7 in strcpy /local/mnt/workspace/bcain_clang_vm-bcain-aus_3184/final/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:423:5
    #1 0x55492e in lou_logFile /opt/disk/marsman/liblouis/6223f21/build_asan/liblouis/../../code/liblouis/logging.c:130:34
    #2 0x553634 in AFG_func /opt/disk/marsman/liblouis/6223f21/./driver-API-6223f21-lou_logFile-BO.c:17:2
    #3 0x553850 in LLVMFuzzerTestOneInput /opt/disk/marsman/liblouis/6223f21/./driver-API-6223f21-lou_logFile-BO.c:44:2
    #4 0x459911 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /local/mnt/workspace/bcain_clang_vm-bcain-aus_3184/final/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
    #5 0x4435d2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /local/mnt/workspace/bcain_clang_vm-bcain-aus_3184/final/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:323:6
    #6 0x449940 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /local/mnt/workspace/bcain_clang_vm-bcain-aus_3184/final/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:856:9
    #7 0x4738c2 in main /local/mnt/workspace/bcain_clang_vm-bcain-aus_3184/final/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #8 0x7f8ea559983f in __libc_start_main /build/glibc-S7Ft5T/glibc-2.23/csu/../csu/libc-start.c:291
    #9 0x41e0d8 in _start (/opt/disk/marsman/liblouis/6223f21/driver-API-6223f21-lou_logFile-BO+0x41e0d8)

0x00000108b6e0 is located 0 bytes to the right of global variable 'initialLogFileName' defined in '../../code/liblouis/logging.c:121:13' (0x108b5e0) of size 256
SUMMARY: AddressSanitizer: global-buffer-overflow /local/mnt/workspace/bcain_clang_vm-bcain-aus_3184/final/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:423:5 in strcpy
Shadow bytes around the buggy address:
  0x000080209680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9
  0x000080209690: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
  0x0000802096a0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
  0x0000802096b0: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x0000802096c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0000802096d0: 00 00 00 00 00 00 00 00 00 00 00 00[f9]f9 f9 f9
  0x0000802096e0: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000802096f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080209700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080209710: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080209720: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
  Shadow gap:              cc
==20412==ABORTING

[Marsman1996]

I checked other strcpy() functions in this library, they are OK since the size of the src and the dest buffer is equal.

Except for the strcpy() in print_dots() at lou_trace.c:

liblouis/tools/lou_trace.c

Lines 100 to 105 in 018edce

	static char *
	print_dots(const widechar *buffer, int length) {
	static char dots[BUFSIZE];
	strcpy(dots, _lou_showDots(buffer, length));
	return dots;
	}

The max size of the src buffer (i.e., scratchBuf) returned by _lou_showDots() is 2048.

liblouis/liblouis/utils.c

Lines 260 to 275 in 1cf7687

	_lou_showDots(widechar const *dots, int length) {
	int bufPos = 0;
	static char scratchBuf[MAXSTRING];
	for (int dotsPos = 0; dotsPos < length && bufPos < (MAXSTRING - 1); dotsPos++) {
	for (int mappingPos = 0; dotMapping[mappingPos].key; mappingPos++) {
	if ((dots[dotsPos] & dotMapping[mappingPos].key) &&
	(bufPos < (MAXSTRING - 1)))
	scratchBuf[bufPos++] = dotMapping[mappingPos].value;
	}
	if ((dots[dotsPos] == LOU_DOTS) && (bufPos < (MAXSTRING - 1)))
	scratchBuf[bufPos++] = '0';
	if ((dotsPos != length - 1) && (bufPos < (MAXSTRING - 1)))
	scratchBuf[bufPos++] = '-';
	}
	scratchBuf[bufPos] = 0;
	return scratchBuf;

While the size of the dest buffer (i.e., dots) is 512.

However, the above is my static analysis result, and for now I cannot trigger the overflow from the program or API entry.
So maybe it is a false positive.

[Marsman1996]

Should I commit the fix together with #1300, or open a new pull request?

[egli]

Should I commit the fix together with #1300, or open a new pull request?

Hi @Marsman1996 , please open a new PR for this issue, thanks

[Marsman1996]

Hi, I open a new PR #1302 to fix this problem.