Skip to content

Commit

Permalink
fix(scap): set a null terminator when we collect args from /proc
Browse files Browse the repository at this point in the history
Signed-off-by: Andrea Terzolo <[email protected]>
  • Loading branch information
Andreagit97 authored and poiana committed May 2, 2024
1 parent d77ceb1 commit 1df2a53
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 11 deletions.
30 changes: 20 additions & 10 deletions userspace/libscap/linux/scap_procs.c
Original file line number Diff line number Diff line change
Expand Up @@ -658,26 +658,35 @@ static int32_t scap_proc_add_from_proc(struct scap_linux_platform* linux_platfor
{
ASSERT(sizeof(line) >= SCAP_MAX_ARGS_SIZE);

filesize = fread(line, 1, SCAP_MAX_ARGS_SIZE - 1, f);
filesize = fread(line, 1, SCAP_MAX_ARGS_SIZE, f);
if(filesize > 0)
{
line[filesize] = 0;
// In case `args` is greater than `SCAP_MAX_ARGS_SIZE` it could be
// truncated so we put a `/0` at the end manually.
line[filesize-1] = 0;

exe_len = strlen(line);
if(exe_len < filesize)
{
++exe_len;
}
// We always count also the terminator so `+1`
// Please note that this could be exactly `SCAP_MAX_ARGS_SIZE`
exe_len = strlen(line) + 1;

snprintf(tinfo.exe, SCAP_MAX_PATH_SIZE, "%s", line);

// Please note if `exe_len` is `SCAP_MAX_ARGS_SIZE` we will return an empty `args`.
tinfo.args_len = filesize - exe_len;

memcpy(tinfo.args, line + exe_len, tinfo.args_len);
tinfo.args[SCAP_MAX_ARGS_SIZE - 1] = 0;
if(tinfo.args_len > 0)
{
memcpy(tinfo.args, line + exe_len, tinfo.args_len);
tinfo.args[tinfo.args_len - 1] = 0;
}
else
{
tinfo.args_len = 0;
tinfo.args[0] = 0;
}
}
else
{
tinfo.args_len = 0;
tinfo.args[0] = 0;
tinfo.exe[0] = 0;
}
Expand Down Expand Up @@ -713,6 +722,7 @@ static int32_t scap_proc_add_from_proc(struct scap_linux_platform* linux_platfor
else
{
tinfo.env[0] = 0;
tinfo.env_len = 0;
}

fclose(f);
Expand Down
12 changes: 12 additions & 0 deletions userspace/libsinsp/test/sinsp_utils.ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,15 @@ TEST(sinsp_utils_test, sinsp_split_buf)
EXPECT_EQ(split[0], "hello");
EXPECT_EQ(split[1], "world");
}

TEST(sinsp_utils_test, sinsp_split_check_terminator)
{
// check that the null terminator is enforced
const char *in = "hello\0worlddd";
size_t len = 13;
auto split = sinsp_split(in, len, '\0');

EXPECT_EQ(split.size(), 2);
EXPECT_EQ(split[0], "hello");
EXPECT_EQ(split[1], "worldd");
}
9 changes: 8 additions & 1 deletion userspace/libsinsp/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1392,14 +1392,21 @@ std::vector<std::string> sinsp_split(const char *buf, size_t len, char delim)
return {};
}

std::string s {buf, len - 1};

if(buf[len - 1] != '\0')
{
#ifdef _DEBUG
throw sinsp_exception("expected a NUL-terminated buffer of size " +
std::to_string(len) + ", which instead ends with " +
std::to_string(buf[len - 1]));
#else
libsinsp_logger()->format(sinsp_logger::SEV_WARNING, "expected a NUL-terminated buffer of size '%ld' which instead ends with '%c'", len, buf[len - 1]);
// enforce the null terminator
s.replace(len-1, 1, "\0");
#endif
}

std::string s {buf, len - 1};
return sinsp_split(s, delim);
}

Expand Down

0 comments on commit 1df2a53

Please sign in to comment.