Skip to content

Commit

Permalink
Port str_comp_filenames from ddnet to fix buffer overflow
Browse files Browse the repository at this point in the history
Fixes a bug caught by clang tooling.

Running for example ``str_comp_filenames("historic_maps/cb_cp/ChillBlock5", "historic_maps/cb_cp/ChillBlock5");``
causes the following error:

```
=================================================================
==108107==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe71e6aa00 at pc 0x55be3668c69c bp 0x7ffe71e6a780 sp 0x7ffe71e6a778
READ of size 1 at 0x7ffe71e6aa00 thread T0
    #0 0x55be3668c69b in str_comp_filenames /home/chiller/Desktop/git/teeworlds/src/base/system.c:2369:14
    #1 0x55be363a0f2f in CServer::CMapListEntry::operator<(CServer::CMapListEntry const&) const /home/chiller/Desktop/git/teeworlds/src/engine/server/server.h:184:61
    #2 0x55be3639f4e7 in plain_range_sorted<CServer::CMapListEntry> partition_binary<plain_range_sorted<CServer::CMapListEntry>, CServer::CMapListEntry>(plain_range_sorted<CServer::CMapListEntry>, CServer::CMapListEntry) /home/chiller/Desktop/git/teeworlds/src/base/tl/algorithm.h:51:25
    #3 0x55be3639a9f1 in sorted_array<CServer::CMapListEntry, allocator_default<CServer::CMapListEntry> >::add(CServer::CMapListEntry const&) /home/chiller/Desktop/git/teeworlds/src/base/tl/sorted_array.h:23:31
    #4 0x55be3637d7d1 in CServer::MapListEntryCallback(char const*, int, int, void*) /home/chiller/Desktop/git/teeworlds/src/engine/server/server.cpp:1504:17
    #5 0x55be36682aba in fs_listdir /home/chiller/Desktop/git/teeworlds/src/base/system.c:1583:6
    #6 0x55be36649f3f in CStorage::ListDirectory(int, char const*, int (*)(char const*, int, int, void*), void*) /home/chiller/Desktop/git/teeworlds/src/engine/shared/storage.cpp:299:5
    #7 0x55be3637d15a in CServer::MapListEntryCallback(char const*, int, int, void*) /home/chiller/Desktop/git/teeworlds/src/engine/server/server.cpp:1492:22
    #8 0x55be36682aba in fs_listdir /home/chiller/Desktop/git/teeworlds/src/base/system.c:1583:6
    #9 0x55be36649f3f in CStorage::ListDirectory(int, char const*, int (*)(char const*, int, int, void*), void*) /home/chiller/Desktop/git/teeworlds/src/engine/shared/storage.cpp:299:5
    #10 0x55be3637d15a in CServer::MapListEntryCallback(char const*, int, int, void*) /home/chiller/Desktop/git/teeworlds/src/engine/server/server.cpp:1492:22
    #11 0x55be36682aba in fs_listdir /home/chiller/Desktop/git/teeworlds/src/base/system.c:1583:6
    #12 0x55be36649f3f in CStorage::ListDirectory(int, char const*, int (*)(char const*, int, int, void*), void*) /home/chiller/Desktop/git/teeworlds/src/engine/shared/storage.cpp:299:5
    #13 0x55be3637c031 in CServer::InitMapList() /home/chiller/Desktop/git/teeworlds/src/engine/server/server.cpp:1523:14
    #14 0x55be36373bb0 in CServer::Run() /home/chiller/Desktop/git/teeworlds/src/engine/server/server.cpp:1281:2
    #15 0x55be3638e83b in main /home/chiller/Desktop/git/teeworlds/src/engine/server/server.cpp:1909:21
    #16 0x7fec6e2461c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #17 0x7fec6e246284 in __libc_start_main csu/../csu/libc-start.c:360:3
    #18 0x55be36279aa0 in _start (/home/chiller/Desktop/git/teeworlds/build/teeworlds_srv+0x1dfaa0) (BuildId: 9b8cae111ee49e0bd87b22dbd9f66cf8ddfb1d52)

Address 0x7ffe71e6aa00 is located in stack of thread T0 at offset 64 in frame
    #0 0x55be3639f0ff in plain_range_sorted<CServer::CMapListEntry> partition_binary<plain_range_sorted<CServer::CMapListEntry>, CServer::CMapListEntry>(plain_range_sorted<CServer::CMapListEntry>, CServer::CMapListEntry) /home/chiller/Desktop/git/teeworlds/src/base/tl/algorithm.h:36

  This frame has 5 object(s):
    [32, 64) 'value.byval' <== Memory access at offset 64 overflows this variable
    [96, 112) 'retval'
    [128, 144) 'range'
    [160, 176) 'ref.tmp' (line 52)
    [192, 208) 'ref.tmp20' (line 54)
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 /home/chiller/Desktop/git/teeworlds/src/base/system.c:2369:14 in str_comp_filenames
Shadow bytes around the buggy address:
  0x10004e3c54f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10004e3c5500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10004e3c5510: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10004e3c5520: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10004e3c5530: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
=>0x10004e3c5540:[f2]f2 f2 f2 00 00 f2 f2 00 00 f2 f2 f8 f8 f2 f2
  0x10004e3c5550: f8 f8 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
  0x10004e3c5560: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10004e3c5570: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10004e3c5580: 00 00 00 00 f1 f1 f1 f1 00 00 f2 f2 00 00 f2 f2
  0x10004e3c5590: 00 00 f2 f2 00 00 00 00 f3 f3 f3 f3 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
==108107==ABORTING
```
  • Loading branch information
ChillerDragon committed Feb 23, 2024
1 parent a1911c8 commit 3f5016d
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions src/base/system.c
Original file line number Diff line number Diff line change
Expand Up @@ -2375,22 +2375,23 @@ int str_comp_filenames(const char *a, const char *b)
{
if(!result)
result = *a - *b;
++a; ++b;
}
while(*a >= '0' && *a <= '9' && *b >= '0' && *b <= '9');
++a;
++b;
} while(*a >= '0' && *a <= '9' && *b >= '0' && *b <= '9');

if(*a >= '0' && *a <= '9')
return 1;
else if(*b >= '0' && *b <= '9')
return -1;
else if(result)
else if(result || *a == '\0' || *b == '\0')
return result;
}

if(tolower(*a) != tolower(*b))
break;
result = tolower(*a) - tolower(*b);
if(result)
return result;
}
return tolower(*a) - tolower(*b);
return *a - *b;
}

const char *str_startswith_nocase(const char *str, const char *prefix)
Expand Down

0 comments on commit 3f5016d

Please sign in to comment.