From 0726d39cb1e257ba70666862f6380537019d8b94 Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Mon, 3 May 2021 00:15:11 -0700 Subject: [PATCH] LibC: Fix invalid 1-byte read I introduced in dirent. When attempting to fix the dirent code I also changed this to use strlcpy instead of the custom string copy loop that was there before. Looking over strlcpy it looked like it should work when using a non null terminated string, I obviously misinterpreted the implementation as it will read till it finds a null terminator. Manually null terminate the string to address this. Gunnar found this after he fixed UserspaceEmulator. I reproduced it locally using his branch, and also found the memory leak I had in the unit test for the scandir that I added, so lets fix that as well. Reported-by: Gunnar Beutner --- Userland/Libraries/LibC/dirent.cpp | 9 +++++++-- Userland/Tests/LibC/TestLibCDirEnt.cpp | 1 - 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Userland/Libraries/LibC/dirent.cpp b/Userland/Libraries/LibC/dirent.cpp index 7002be78b9..67688bc584 100644 --- a/Userland/Libraries/LibC/dirent.cpp +++ b/Userland/Libraries/LibC/dirent.cpp @@ -72,8 +72,13 @@ static void create_struct_dirent(sys_dirent* sys_ent, struct dirent* str_ent) str_ent->d_off = 0; str_ent->d_reclen = sizeof(struct dirent); - int size = min((sys_ent->namelen + 1) * sizeof(char), sizeof(str_ent->d_name)); - [[maybe_unused]] auto n = strlcpy(str_ent->d_name, sys_ent->name, size); + VERIFY(sizeof(str_ent->d_name) > sys_ent->namelen); + + // Note: We can't use any normal string function as sys_ent->name is + // not null terminated. All string copy functions will attempt to read + // the non-existent null terminator past the end of the source string. + memcpy(str_ent->d_name, sys_ent->name, sys_ent->namelen); + str_ent->d_name[sys_ent->namelen] = '\0'; } static int allocate_dirp_buffer(DIR* dirp) diff --git a/Userland/Tests/LibC/TestLibCDirEnt.cpp b/Userland/Tests/LibC/TestLibCDirEnt.cpp index b3c7a2e171..b7f85d7528 100644 --- a/Userland/Tests/LibC/TestLibCDirEnt.cpp +++ b/Userland/Tests/LibC/TestLibCDirEnt.cpp @@ -19,7 +19,6 @@ TEST_CASE(scandir_basic_scenario) for (auto i = 0; i < entries; i++) { if (strcmp(namelist[i]->d_name, "passwd") == 0) { found_passwd = true; - break; } free(namelist[i]); }