Stack smashing when handling multiple csv files.

[Junyi-99]

I wanted to thank you for your contribution to the FedTree project. However, I noticed an issue with the code that I would like to bring to your attention.

As mentioned in issue #58, there seems to be an undefined behavior in the code related to stack smashing. While the commit(https://github.com/Xtra-Computing/FedTree/commit/376c7de020a409a6a64d9acbe127dc451e8a978c) you made may have mitigated the problem, it did not solve it completely.

TL;DR

The name and val were not initialized to zero causing unauthorized memory access.

Details

After some debugging of the code, I found that the issue is located in parser.cpp:81. https://github.com/Xtra-Computing/FedTree/blob/a3461e14023d1b86aacc1d50bf705b18125ee7a9/src/FedTree/parser.cpp#L81 where the arrays name and val are not initialized to zero, and thus contain garbage information (in some OS). This causes unauthorized memory access when the string str_name(name); https://github.com/Xtra-Computing/FedTree/blob/a3461e14023d1b86aacc1d50bf705b18125ee7a9/src/FedTree/parser.cpp#L184 constructor of the std::string class seeks characters until it meets a \0 character. I appreciate your contribution to the project and hope this information helps to fix the issue.

I hope this information helps you to fix the issue. Thank you again for your contribution to the project.

[QinbinLi]

Hi @Junyi-99 ,

Thanks a lot for your information! I have fixed it accordingly.

[Junyi-99]

@QinbinLi Thanks for the reply, I initialized the variable but the error still occur. Here is the sanitizer's output.

=================================================================
==1832572==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffeb3e84970 at pc 0x7f4144c014fd bp 0x7ffeb3e83660 sp 0x7ffeb3e82de8
WRITE of size 2102 at 0x7ffeb3e84970 thread T0
    #0 0x7f4144c014fc in scanf_common ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:341
    #1 0x7f4144c0286f in __interceptor___isoc99_vsscanf ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1474
    #2 0x7f4144c0297e in __interceptor___isoc99_sscanf ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1497
    #3 0x7f4144a72f53 in operator() /home/junyi/VertiBench-master/algo/FedTree/src/FedTree/parser.cpp:84
    #4 0x7f4144a72f53 in Parser::parse_param(FLParam&, char*) /home/junyi/VertiBench-master/algo/FedTree/src/FedTree/parser.cpp:206
    #5 0x55d89e90fccb in main /home/junyi/VertiBench-master/algo/FedTree/src/FedTree/fedtree_train.cpp:25
    #6 0x7f4143e98082 in __libc_start_main ../csu/libc-start.c:308
    #7 0x55d89e91a04d in _start (/data/junyi/zhaomin/algo/FedTree/build/bin/FedTree-train+0x1304d)

Address 0x7ffeb3e84970 is located in stack of thread T0 at offset 4288 in frame
    #0 0x7f4144a71cef in Parser::parse_param(FLParam&, char*) /home/junyi/VertiBench-master/algo/FedTree/src/FedTree/parser.cpp:29

  This frame has 33 object(s):
    [32, 33) '<unknown>'
    [48, 49) '<unknown>'
    [64, 65) '<unknown>'
    [80, 81) '<unknown>'
    [96, 97) '<unknown>'
    [112, 113) '<unknown>'
    [128, 129) '<unknown>'
    [144, 145) '<unknown>'
    [160, 161) '<unknown>'
    [176, 177) '<unknown>'
    [192, 193) '<unknown>'
    [208, 209) '<unknown>'
    [224, 225) '<unknown>'
    [240, 241) '<unknown>'
    [256, 257) '<unknown>'
    [272, 280) 'gbdt_param' (line 53)
    [304, 312) '__dnew'
    [336, 344) '__dnew'
    [368, 384) 'parse_value' (line 80)
    [400, 424) '<unknown>'
    [464, 488) '<unknown>'
    [528, 640) '<unknown>'
    [672, 784) '<unknown>'
    [816, 848) 'line' (line 202)
    [880, 912) 'str_name' (line 85)
    [944, 976) 'str_name' (line 186)
    [1008, 1040) '<unknown>'
    [1072, 1104) '<unknown>'
    [1136, 1168) '<unknown>'
    [1200, 1232) '<unknown>'
    [1264, 1784) 'conf_file' (line 201)
    [1920, 2176) 'name' (line 81)
    [2240, 4288) 'val' (line 81) <== Memory access at offset 4288 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:341 in scanf_common
Shadow bytes around the buggy address:
  0x1000567c88d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000567c88e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000567c88f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000567c8900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000567c8910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x1000567c8920: 00 00 00 00 00 00 00 00 00 00 00 00 00 00[f3]f3
  0x1000567c8930: f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 00 00
  0x1000567c8940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000567c8950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000567c8960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000567c8970: 00 00 f1 f1 f1 f1 f8 f2 01 f2 01 f2 01 f2 01 f2
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
==1832572==ABORTING

[Junyi-99]

I know, it's because the files are too many and exceeds the val[2048]. Increasing array size to 81920 solved the problem.