From 2ee5027ad7d9cd073ea2428a39c693f9c8963bfc Mon Sep 17 00:00:00 2001 From: haozi007 Date: Fri, 13 May 2022 10:31:02 +0100 Subject: [PATCH 14/16] fix ut bug and arguments check 1. hold_int must return error when invalid digits, ie. "root:x:1a"; 2. hold_string_list return NULL only overflow buffer, other return buff_start; 3. parse_line_gr must check hold_string_list return; 4. must check result argument is not NULL; 5. use '\xff' to check buffer is overflow; Signed-off-by: haozi007 --- .../modules/image/image_rootfs_handler.c | 2 +- src/utils/cutils/utils_pwgr.c | 47 ++++++++++++++----- test/cutils/utils_pwgr/group_sample | 1 + test/cutils/utils_pwgr/passwd_sample | 1 + test/cutils/utils_pwgr/utils_pwgr_ut.cc | 20 +++++--- 5 files changed, 51 insertions(+), 20 deletions(-) diff --git a/src/daemon/modules/image/image_rootfs_handler.c b/src/daemon/modules/image/image_rootfs_handler.c index 960d52c7..ac1afdc6 100644 --- a/src/daemon/modules/image/image_rootfs_handler.c +++ b/src/daemon/modules/image/image_rootfs_handler.c @@ -84,7 +84,7 @@ static int proc_by_fpasswd(FILE *f_passwd, const char *user, defs_process_user * int uret = -1; bool userfound = false; long long n_user = 0; - char buf[BUFSIZ]; + char buf[BUFSIZ] = { 0 }; struct passwd pw; struct passwd *pwbufp = NULL; diff --git a/src/utils/cutils/utils_pwgr.c b/src/utils/cutils/utils_pwgr.c index f4588268..17cf017b 100644 --- a/src/utils/cutils/utils_pwgr.c +++ b/src/utils/cutils/utils_pwgr.c @@ -57,6 +57,10 @@ static int hold_int(const char delim, bool required, char **src, unsigned int *d } res = strtoll(*src, &err_str, 0); + if (err_str == *src) { + ERROR("invalid digits string."); + return -1; + } if (errno == ERANGE) { ERROR("Parse int from string failed."); return -1; @@ -158,9 +162,6 @@ static char **hold_string_list(char **line, char *buf_start, char *buf_end, cons char **result = NULL; char **walker = NULL; - if (**line == '\0') { - return 0; - } // For ultimate space usage, the blank area from buffer which was allocated from stack is used buf_start += __alignof__(char *) - 1; // align the starting position of the buffer to use it as a 2d array @@ -174,7 +175,7 @@ static char **hold_string_list(char **line, char *buf_start, char *buf_end, cons (void)util_trim_space(*line); if (hold_string(',', line, walker) != 0) { ERROR("Parse string list error."); - return NULL; + return result; } if ((char *)(walker + 2) > buf_end) { @@ -218,6 +219,10 @@ static int parse_line_gr(const char delim, char *line, size_t buflen, struct gro } result->gr_mem = hold_string_list(&line, freebuff, buffend, ','); + if (result->gr_mem == NULL) { + ERROR("overflow of buffer."); + return -1; + } return 0; } @@ -225,8 +230,10 @@ static int parse_line_gr(const char delim, char *line, size_t buflen, struct gro int util_getpwent_r(FILE *stream, struct passwd *resbuf, char *buffer, size_t buflen, struct passwd **result) { const char delim = ':'; + char *buff_end = NULL; + bool got = false; - if (stream == NULL || resbuf == NULL || buffer == NULL) { + if (stream == NULL || resbuf == NULL || buffer == NULL || result == NULL) { ERROR("Password obj, params is NULL."); return -1; } @@ -242,28 +249,34 @@ int util_getpwent_r(FILE *stream, struct passwd *resbuf, char *buffer, size_t bu } __fsetlocking(stream, FSETLOCKING_BYCALLER); - buffer[buflen - 1] = '\0'; if (feof(stream)) { *result = NULL; return ENOENT; } + buff_end = buffer + buflen - 1; - while (fgets(buffer, buflen, stream) != NULL) { + while ((*buff_end = '\xff') && fgets(buffer, buflen, stream) != NULL) { (void)util_trim_space(buffer); if (buffer[0] == '\0' || buffer[0] == '#' || strlen(buffer) < 1) { continue; } if (parse_line_pw(delim, buffer, resbuf) == 0) { + got = true; break; } - if (buffer[buflen - 1] != '\0') { + if (*buff_end != '\xff') { *result = NULL; return ERANGE; } } + if (!got) { + *result = NULL; + return ERANGE; + } + *result = resbuf; return 0; @@ -272,8 +285,10 @@ int util_getpwent_r(FILE *stream, struct passwd *resbuf, char *buffer, size_t bu int util_getgrent_r(FILE *stream, struct group *resbuf, char *buffer, size_t buflen, struct group **result) { const char delim = ':'; + char *buff_end = NULL; + bool got = false; - if (stream == NULL || resbuf == NULL || buffer == NULL) { + if (stream == NULL || resbuf == NULL || buffer == NULL || result == NULL) { ERROR("Group obj, params is NULL."); return -1; } @@ -289,29 +304,35 @@ int util_getgrent_r(FILE *stream, struct group *resbuf, char *buffer, size_t buf } __fsetlocking(stream, FSETLOCKING_BYCALLER); - buffer[buflen - 1] = '\0'; if (feof(stream)) { *result = NULL; return ENOENT; } + buff_end = buffer + buflen - 1; - while (fgets(buffer, buflen, stream) != NULL) { + while ((*buff_end = '\xff') && fgets(buffer, buflen, stream) != NULL) { (void)util_trim_space(buffer); if (buffer[0] == '\0' || buffer[0] == '#' || strlen(buffer) < 1) { continue; } if (parse_line_gr(delim, buffer, buflen, resbuf) == 0) { + got = true; break; } - if (buffer[buflen - 1] != '\0') { + if (*buff_end != '\xff') { *result = NULL; return ERANGE; } } - *result = resbuf; + if (!got) { + *result = NULL; + return ERANGE; + } + + *result = resbuf; return 0; } \ No newline at end of file diff --git a/test/cutils/utils_pwgr/group_sample b/test/cutils/utils_pwgr/group_sample index c73883cc..8de62df2 100644 --- a/test/cutils/utils_pwgr/group_sample +++ b/test/cutils/utils_pwgr/group_sample @@ -1,3 +1,4 @@ +root:x:a root:x:0: #bin:x:1: diff --git a/test/cutils/utils_pwgr/passwd_sample b/test/cutils/utils_pwgr/passwd_sample index 76853454..995dea3a 100644 --- a/test/cutils/utils_pwgr/passwd_sample +++ b/test/cutils/utils_pwgr/passwd_sample @@ -1,3 +1,4 @@ +root:x:b:0: root:x:0:0:root:/root:/bin/bash bin:x:1:1:bin:/bin:/sbin/nologin bin:x:-1:1:bin:/bin:/sbin/nologin diff --git a/test/cutils/utils_pwgr/utils_pwgr_ut.cc b/test/cutils/utils_pwgr/utils_pwgr_ut.cc index 1ed8eaa1..d856270d 100644 --- a/test/cutils/utils_pwgr/utils_pwgr_ut.cc +++ b/test/cutils/utils_pwgr/utils_pwgr_ut.cc @@ -25,7 +25,10 @@ TEST(utils_pwgr, test_getpwent_r) struct passwd pw; struct passwd *ppw = nullptr; struct passwd *ppw_alter = &pw; - char buf[BUFSIZ]; + char buf[BUFSIZ] = { 0 }; + char invalid_buf[1] = { 0 }; + // use to get ERANGE error + char small_buf[10] = { 0 }; std::vector> testcase = { std::make_tuple("root", "x", 0, 0, "root", "/root", "/bin/bash"), @@ -39,7 +42,7 @@ TEST(utils_pwgr, test_getpwent_r) ASSERT_EQ(util_getpwent_r(NULL, &pw, buf, sizeof(buf), &ppw), -1); ASSERT_EQ(util_getpwent_r(f_pw, &pw, NULL, 0, &ppw), -1); - ASSERT_EQ(util_getpwent_r(f_pw, &pw, NULL, 1, &ppw), -1); + ASSERT_EQ(util_getpwent_r(f_pw, &pw, invalid_buf, 1, &ppw), -1); ASSERT_EQ(util_getpwent_r(f_pw, &pw, buf, sizeof(buf), &ppw_alter), -1); while (!feof(f_pw)) { @@ -48,6 +51,8 @@ TEST(utils_pwgr, test_getpwent_r) ASSERT_EQ(util_getpwent_r(f_pw, &pw, buf, sizeof(buf), &ppw), ENOENT); rewind(f_pw); + ASSERT_EQ(util_getpwent_r(f_pw, &pw, small_buf, sizeof(small_buf), &ppw), ERANGE); + for (const auto &elem : testcase) { ASSERT_EQ(util_getpwent_r(f_pw, &pw, buf, sizeof(buf), &ppw), 0); ASSERT_STREQ(pw.pw_name, std::get<0>(elem).c_str()); @@ -74,7 +79,10 @@ TEST(utils_pwgr, test_getgrent_r) struct group gr{0}; struct group *pgr = nullptr; struct group *pgr_alter = &gr; - char buf[BUFSIZ]; + char buf[BUFSIZ] = { 0 }; + char invalid_buf[1] = { 0 }; + // use to get ERANGE error + char small_buf[9] = { 0 }; size_t i = 0; size_t j = 0; std::vector> string_list{ @@ -95,7 +103,7 @@ TEST(utils_pwgr, test_getgrent_r) ASSERT_EQ(util_getgrent_r(NULL, &gr, buf, sizeof(buf), &pgr), -1); ASSERT_EQ(util_getgrent_r(f_gr, &gr, NULL, 0, &pgr), -1); - ASSERT_EQ(util_getgrent_r(f_gr, &gr, NULL, 1, &pgr), -1); + ASSERT_EQ(util_getgrent_r(f_gr, &gr, invalid_buf, 1, &pgr), -1); ASSERT_EQ(util_getgrent_r(f_gr, &gr, buf, sizeof(buf), &pgr_alter), -1); while (!feof(f_gr)) { @@ -104,6 +112,8 @@ TEST(utils_pwgr, test_getgrent_r) ASSERT_EQ(util_getgrent_r(f_gr, &gr, buf, sizeof(buf), &pgr), ENOENT); rewind(f_gr); + ASSERT_EQ(util_getgrent_r(f_gr, &gr, small_buf, sizeof(small_buf), &pgr), ERANGE); + for (; i < string_list.size(); ++i) { ASSERT_EQ(util_getgrent_r(f_gr, &gr, buf, sizeof(buf), &pgr), 0); ASSERT_STREQ(gr.gr_name, std::get<0>(testcase[i]).c_str()); @@ -113,8 +123,6 @@ TEST(utils_pwgr, test_getgrent_r) for (j = 0; j < string_list[i].size(); ++j) { EXPECT_TRUE(strcmp(gr.gr_mem[j], string_list[i][j].c_str()) == 0); } - } else { - EXPECT_TRUE(gr.gr_mem == nullptr); } EXPECT_TRUE(pgr == &gr); gr = {0}; -- 2.20.1