Global Buffer Overflow in the trimname() function

Hi,

I found a global buffer overflow write of size 1 in the trimname() function, in main.c#L739 on dpic 2021.04.10 (commit: d317e406)

As per the function code, trimname(),

Char *
trimname (Char * fn, int len) {
  static Char fnbuf[256];
  Char *cp = fnbuf;
  while (--len >= 0 && *fn && !isspace ((unsigned)*fn))
    *cp++ = *fn++; //Line no. 739
  *cp = 0;
  return fnbuf;
}

Based on the investigation, the error is in the precedence of operators, as *cp++ works like *(cp++) and len is 1024, mentioned in the function pointoutput() in main.c#L826 which is sizeof(mstring) and further, it is typedef Char string[FILENAMELEN] and FILENAMELEN is 1024, so cp will go from 1 to 256 and causes write of size 1 as the loop will go from 1023 to 0 but the cp is of size 256.

Improvement suggestions:

  • increase the fnbuf[256] to fnbuf[FILENAMELEN] as it loops over sizeof(mstring) which is FILENAMELEN
  • also, as the array starts from 0 and here len variable decrements from 1023 to 0, so we have to modify the code line *cp++ = *fn++ and proceed like first assign then increment.
=================================================================
==1427955==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000ed15a0 at pc 0x0000004d063f bp 0x7ffd17805540 sp 0x7ffd17805538
WRITE of size 1 at 0x000000ed15a0 thread T0
    #0 0x4d063e in trimname /home/bsdboy/projects/re-invest/dpic/main.c:739:11
    #1 0x4d1716 in pointoutput /home/bsdboy/projects/re-invest/dpic/main.c:826:33
    #2 0x4ebbfa in yyparse /home/bsdboy/projects/re-invest/dpic/dpic.y:1188:20
    #3 0x4dbcb4 in main /home/bsdboy/projects/re-invest/dpic/main.c:1811:13
    #4 0x7f261ce9c0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #5 0x41c65d in _start (/home/bsdboy/projects/re-invest/dpic/dpic+0x41c65d)

0x000000ed15a0 is located 0 bytes to the right of global variable 'fnbuf' defined in 'main.c:736:15' (0xed14a0) of size 256
SUMMARY: AddressSanitizer: global-buffer-overflow /home/bsdboy/projects/re-invest/dpic/main.c:739:11 in trimname
Shadow bytes around the buggy address:
  0x0000801d2260: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 01 f9 f9 f9
  0x0000801d2270: f9 f9 f9 f9 01 f9 f9 f9 f9 f9 f9 f9 01 f9 f9 f9
  0x0000801d2280: f9 f9 f9 f9 02 f9 f9 f9 f9 f9 f9 f9 02 f9 f9 f9
  0x0000801d2290: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000801d22a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0000801d22b0: 00 00 00 00[f9]f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x0000801d22c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000801d22d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000801d22e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000801d22f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000801d2300: 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
==1427955==ABORTING