diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/responder/pam/pamsrv_cmd.c | 151 | ||||
-rw-r--r-- | src/tests/util-tests.c | 14 | ||||
-rw-r--r-- | src/util/util.h | 5 |
3 files changed, 94 insertions, 76 deletions
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 6a8f1dbb..bb42f712 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -42,18 +42,15 @@ enum pam_verbosity { static void pam_reply(struct pam_auth_req *preq); -static int extract_authtok(uint32_t *type, uint32_t *size, uint8_t **tok, uint8_t *body, size_t blen, size_t *c) { - uint32_t data_size; +static int extract_authtok(uint32_t *type, uint32_t *size, uint8_t **tok, + size_t data_size, uint8_t *body, size_t blen, + size_t *c) { - if (blen-(*c) < 2*sizeof(uint32_t)) return EINVAL; - - memcpy(&data_size, &body[*c], sizeof(uint32_t)); - *c += sizeof(uint32_t); - if (data_size < sizeof(uint32_t) || (*c)+(data_size) > blen) return EINVAL; + if (data_size < sizeof(uint32_t) || *c+data_size > blen || + SIZE_T_OVERFLOW(*c, data_size)) return EINVAL; *size = data_size - sizeof(uint32_t); - memcpy(type, &body[*c], sizeof(uint32_t)); - *c += sizeof(uint32_t); + SAFEALIGN_COPY_UINT32_CHECK(type, &body[*c], blen, c); *tok = body+(*c); @@ -62,15 +59,11 @@ static int extract_authtok(uint32_t *type, uint32_t *size, uint8_t **tok, uint8_ return EOK; } -static int extract_string(char **var, uint8_t *body, size_t blen, size_t *c) { - uint32_t size; +static int extract_string(char **var, size_t size, uint8_t *body, size_t blen, + size_t *c) { uint8_t *str; - if (blen-(*c) < sizeof(uint32_t)+1) return EINVAL; - - memcpy(&size, &body[*c], sizeof(uint32_t)); - *c += sizeof(uint32_t); - if (*c+size > blen) return EINVAL; + if (*c+size > blen || SIZE_T_OVERFLOW(*c, size)) return EINVAL; str = body+(*c); @@ -83,16 +76,13 @@ static int extract_string(char **var, uint8_t *body, size_t blen, size_t *c) { return EOK; } -static int extract_uint32_t(uint32_t *var, uint8_t *body, size_t blen, size_t *c) { - uint32_t size; - - if (blen-(*c) < 2*sizeof(uint32_t)) return EINVAL; +static int extract_uint32_t(uint32_t *var, size_t size, uint8_t *body, + size_t blen, size_t *c) { - memcpy(&size, &body[*c], sizeof(uint32_t)); - *c += sizeof(uint32_t); + if (size != sizeof(uint32_t) || *c+size > blen || SIZE_T_OVERFLOW(*c, size)) + return EINVAL; - memcpy(var, &body[*c], sizeof(uint32_t)); - *c += sizeof(uint32_t); + SAFEALIGN_COPY_UINT32_CHECK(var, &body[*c], blen, c); return EOK; } @@ -117,59 +107,66 @@ static int pam_parse_in_data_v2(struct sss_names_ctx *snctx, c = sizeof(uint32_t); do { - memcpy(&type, &body[c], sizeof(uint32_t)); - c += sizeof(uint32_t); - if (c > blen) return EINVAL; - - switch(type) { - case SSS_PAM_ITEM_USER: - ret = extract_string(&pam_user, body, blen, &c); - if (ret != EOK) return ret; - - ret = sss_parse_name(pd, snctx, pam_user, - &pd->domain, &pd->user); - if (ret != EOK) return ret; - break; - case SSS_PAM_ITEM_SERVICE: - ret = extract_string(&pd->service, body, blen, &c); - if (ret != EOK) return ret; - break; - case SSS_PAM_ITEM_TTY: - ret = extract_string(&pd->tty, body, blen, &c); - if (ret != EOK) return ret; - break; - case SSS_PAM_ITEM_RUSER: - ret = extract_string(&pd->ruser, body, blen, &c); - if (ret != EOK) return ret; - break; - case SSS_PAM_ITEM_RHOST: - ret = extract_string(&pd->rhost, body, blen, &c); - if (ret != EOK) return ret; - break; - case SSS_PAM_ITEM_CLI_PID: - ret = extract_uint32_t(&pd->cli_pid, - body, blen, &c); - if (ret != EOK) return ret; - break; - case SSS_PAM_ITEM_AUTHTOK: - ret = extract_authtok(&pd->authtok_type, &pd->authtok_size, - &pd->authtok, body, blen, &c); - if (ret != EOK) return ret; - break; - case SSS_PAM_ITEM_NEWAUTHTOK: - ret = extract_authtok(&pd->newauthtok_type, - &pd->newauthtok_size, - &pd->newauthtok, body, blen, &c); - if (ret != EOK) return ret; - break; - case SSS_END_OF_PAM_REQUEST: - if (c != blen) return EINVAL; - break; - default: - DEBUG(1,("Ignoring unknown data type [%d].\n", type)); - size = ((uint32_t *)&body[c])[0]; - c += size+sizeof(uint32_t); + SAFEALIGN_COPY_UINT32_CHECK(&type, &body[c], blen, &c); + + if (type == SSS_END_OF_PAM_REQUEST) { + if (c != blen) return EINVAL; + } else { + SAFEALIGN_COPY_UINT32_CHECK(&size, &body[c], blen, &c); + /* the uint32_t end maker SSS_END_OF_PAM_REQUEST does not count to + * the remaining buffer */ + if (size > (blen - c - sizeof(uint32_t))) { + DEBUG(1, ("Invalid data size.\n")); + return EINVAL; + } + + switch(type) { + case SSS_PAM_ITEM_USER: + ret = extract_string(&pam_user, size, body, blen, &c); + if (ret != EOK) return ret; + + ret = sss_parse_name(pd, snctx, pam_user, + &pd->domain, &pd->user); + if (ret != EOK) return ret; + break; + case SSS_PAM_ITEM_SERVICE: + ret = extract_string(&pd->service, size, body, blen, &c); + if (ret != EOK) return ret; + break; + case SSS_PAM_ITEM_TTY: + ret = extract_string(&pd->tty, size, body, blen, &c); + if (ret != EOK) return ret; + break; + case SSS_PAM_ITEM_RUSER: + ret = extract_string(&pd->ruser, size, body, blen, &c); + if (ret != EOK) return ret; + break; + case SSS_PAM_ITEM_RHOST: + ret = extract_string(&pd->rhost, size, body, blen, &c); + if (ret != EOK) return ret; + break; + case SSS_PAM_ITEM_CLI_PID: + ret = extract_uint32_t(&pd->cli_pid, size, + body, blen, &c); + if (ret != EOK) return ret; + break; + case SSS_PAM_ITEM_AUTHTOK: + ret = extract_authtok(&pd->authtok_type, &pd->authtok_size, + &pd->authtok, size, body, blen, &c); + if (ret != EOK) return ret; + break; + case SSS_PAM_ITEM_NEWAUTHTOK: + ret = extract_authtok(&pd->newauthtok_type, + &pd->newauthtok_size, + &pd->newauthtok, size, body, blen, &c); + if (ret != EOK) return ret; + break; + default: + DEBUG(1,("Ignoring unknown data type [%d].\n", type)); + c += size; + } } + } while(c < blen); if (pd->user == NULL || *pd->user == '\0') return EINVAL; @@ -240,6 +237,7 @@ static int pam_parse_in_data(struct sss_names_ctx *snctx, start += sizeof(uint32_t); pd->authtok_size = (int) body[start]; + if (pd->authtok_size >= blen) return EINVAL; start += sizeof(uint32_t); end = start + pd->authtok_size; @@ -259,6 +257,7 @@ static int pam_parse_in_data(struct sss_names_ctx *snctx, start += sizeof(uint32_t); pd->newauthtok_size = (int) body[start]; + if (pd->newauthtok_size >= blen) return EINVAL; start += sizeof(uint32_t); end = start + pd->newauthtok_size; diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c index cf96f0e3..a98b0c03 100644 --- a/src/tests/util-tests.c +++ b/src/tests/util-tests.c @@ -241,6 +241,19 @@ START_TEST(test_sss_filter_sanitize) } END_TEST +START_TEST(test_size_t_overflow) +{ + fail_unless(!SIZE_T_OVERFLOW(1, 1), "unexpected overflow"); + fail_unless(!SIZE_T_OVERFLOW(SIZE_T_MAX, 0), "unexpected overflow"); + fail_unless(!SIZE_T_OVERFLOW(SIZE_T_MAX-10, 10), "unexpected overflow"); + fail_unless(SIZE_T_OVERFLOW(SIZE_T_MAX, 1), "overflow not detected"); + fail_unless(SIZE_T_OVERFLOW(SIZE_T_MAX, SIZE_T_MAX), + "overflow not detected"); + fail_unless(SIZE_T_OVERFLOW(SIZE_T_MAX, ULLONG_MAX), + "overflow not detected"); + fail_unless(SIZE_T_OVERFLOW(SIZE_T_MAX, -10), "overflow not detected"); +} +END_TEST Suite *util_suite(void) { @@ -250,6 +263,7 @@ Suite *util_suite(void) tcase_add_test (tc_util, test_diff_string_lists); tcase_add_test (tc_util, test_sss_filter_sanitize); + tcase_add_test (tc_util, test_size_t_overflow); tcase_set_timeout(tc_util, 60); suite_add_tcase (s, tc_util); diff --git a/src/util/util.h b/src/util/util.h index f1e11a84..ee229347 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -171,6 +171,11 @@ errno_t set_debug_file_from_fd(const int fd); #define OUT_OF_ID_RANGE(id, min, max) \ (id == 0 || (min && (id < min)) || (max && (id > max))) +#define SIZE_T_MAX ((size_t) -1) + +#define SIZE_T_OVERFLOW(current, add) \ + (((size_t)(add)) > (SIZE_T_MAX - ((size_t)(current)))) + static inline void safealign_memcpy(void *dest, const void *src, size_t n, size_t *counter) { |