Skip to content

Commit 1df2a53

Browse files
Andreagit97poiana
authored andcommitted
fix(scap): set a null terminator when we collect args from /proc
Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
1 parent d77ceb1 commit 1df2a53

File tree

3 files changed

+40
-11
lines changed

3 files changed

+40
-11
lines changed

userspace/libscap/linux/scap_procs.c

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -658,26 +658,35 @@ static int32_t scap_proc_add_from_proc(struct scap_linux_platform* linux_platfor
658658
{
659659
ASSERT(sizeof(line) >= SCAP_MAX_ARGS_SIZE);
660660

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

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

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

674+
// Please note if `exe_len` is `SCAP_MAX_ARGS_SIZE` we will return an empty `args`.
674675
tinfo.args_len = filesize - exe_len;
675-
676-
memcpy(tinfo.args, line + exe_len, tinfo.args_len);
677-
tinfo.args[SCAP_MAX_ARGS_SIZE - 1] = 0;
676+
if(tinfo.args_len > 0)
677+
{
678+
memcpy(tinfo.args, line + exe_len, tinfo.args_len);
679+
tinfo.args[tinfo.args_len - 1] = 0;
680+
}
681+
else
682+
{
683+
tinfo.args_len = 0;
684+
tinfo.args[0] = 0;
685+
}
678686
}
679687
else
680688
{
689+
tinfo.args_len = 0;
681690
tinfo.args[0] = 0;
682691
tinfo.exe[0] = 0;
683692
}
@@ -713,6 +722,7 @@ static int32_t scap_proc_add_from_proc(struct scap_linux_platform* linux_platfor
713722
else
714723
{
715724
tinfo.env[0] = 0;
725+
tinfo.env_len = 0;
716726
}
717727

718728
fclose(f);

userspace/libsinsp/test/sinsp_utils.ut.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,3 +208,15 @@ TEST(sinsp_utils_test, sinsp_split_buf)
208208
EXPECT_EQ(split[0], "hello");
209209
EXPECT_EQ(split[1], "world");
210210
}
211+
212+
TEST(sinsp_utils_test, sinsp_split_check_terminator)
213+
{
214+
// check that the null terminator is enforced
215+
const char *in = "hello\0worlddd";
216+
size_t len = 13;
217+
auto split = sinsp_split(in, len, '\0');
218+
219+
EXPECT_EQ(split.size(), 2);
220+
EXPECT_EQ(split[0], "hello");
221+
EXPECT_EQ(split[1], "worldd");
222+
}

userspace/libsinsp/utils.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1392,14 +1392,21 @@ std::vector<std::string> sinsp_split(const char *buf, size_t len, char delim)
13921392
return {};
13931393
}
13941394

1395+
std::string s {buf, len - 1};
1396+
13951397
if(buf[len - 1] != '\0')
13961398
{
1399+
#ifdef _DEBUG
13971400
throw sinsp_exception("expected a NUL-terminated buffer of size " +
13981401
std::to_string(len) + ", which instead ends with " +
13991402
std::to_string(buf[len - 1]));
1403+
#else
1404+
libsinsp_logger()->format(sinsp_logger::SEV_WARNING, "expected a NUL-terminated buffer of size '%ld' which instead ends with '%c'", len, buf[len - 1]);
1405+
// enforce the null terminator
1406+
s.replace(len-1, 1, "\0");
1407+
#endif
14001408
}
14011409

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

0 commit comments

Comments
 (0)