From 5dbe23e6abd7d2b7e534a66ce1f973e75ff6923690d8282ce074e2347ed839ed Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 18 Jan 2011 10:13:29 +0000 Subject: [PATCH] Accepting request 58670 from home:rhafer:branches:network:ldap OBS-URL: https://build.opensuse.org/request/show/58670 OBS-URL: https://build.opensuse.org/package/show/network:ldap/sssd?expand=0&rev=36 --- ...ate-user-supplied-size-of-data-items.patch | 269 ++++++++++++++++++ ...eck-to-SAFEALIGN_COPY_-_CHECK-macros.patch | 32 +++ sssd.changes | 9 + sssd.spec | 4 + 4 files changed, 314 insertions(+) create mode 100644 0001-Validate-user-supplied-size-of-data-items.patch create mode 100644 0002-Add-overflow-check-to-SAFEALIGN_COPY_-_CHECK-macros.patch diff --git a/0001-Validate-user-supplied-size-of-data-items.patch b/0001-Validate-user-supplied-size-of-data-items.patch new file mode 100644 index 0000000..8355e83 --- /dev/null +++ b/0001-Validate-user-supplied-size-of-data-items.patch @@ -0,0 +1,269 @@ +From af93a65bebb1f007eecbeabd07b7ae8b7cc276c9 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Mon, 6 Dec 2010 21:18:50 +0100 +Subject: Validate user supplied size of data items + +Specially crafted packages might lead to an integer overflow and the +parsing of the input buffer might not continue as expected. This issue +was identified by Sebastian Krahmer . + +bnc#660481 +CVE-2010-4341 + +diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c +index 7bfd0f2..f4fe4f7 100644 +--- a/src/responder/pam/pamsrv_cmd.c ++++ b/src/responder/pam/pamsrv_cmd.c +@@ -33,18 +33,15 @@ + + 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); + +@@ -53,15 +50,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); + +@@ -74,16 +67,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; + } +@@ -108,59 +98,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; +@@ -231,6 +228,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; +@@ -250,6 +248,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 bfc48bb..328ae23 100644 +--- a/src/tests/util-tests.c ++++ b/src/tests/util-tests.c +@@ -175,6 +175,20 @@ START_TEST(test_diff_string_lists) + } + 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) + { + Suite *s = suite_create("util"); +@@ -182,6 +196,7 @@ Suite *util_suite(void) + TCase *tc_util = tcase_create("util"); + + tcase_add_test (tc_util, test_diff_string_lists); ++ 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 e93f6f8..7c35550 100644 +--- a/src/util/util.h ++++ b/src/util/util.h +@@ -169,6 +169,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) + { +-- +1.7.3.2 + diff --git a/0002-Add-overflow-check-to-SAFEALIGN_COPY_-_CHECK-macros.patch b/0002-Add-overflow-check-to-SAFEALIGN_COPY_-_CHECK-macros.patch new file mode 100644 index 0000000..c92a90f --- /dev/null +++ b/0002-Add-overflow-check-to-SAFEALIGN_COPY_-_CHECK-macros.patch @@ -0,0 +1,32 @@ +From bfac6031ab075834183c9f18b28363d11b99e44a Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Tue, 7 Dec 2010 17:01:04 +0100 +Subject: Add overflow check to SAFEALIGN_COPY_*_CHECK macros + +CVE-2010-4341 +bnc#660481 + +diff --git a/src/util/util.h b/src/util/util.h +index 7c35550..50c5fe2 100644 +--- a/src/util/util.h ++++ b/src/util/util.h +@@ -207,12 +207,14 @@ safealign_memcpy(void *dest, const void *src, size_t n, size_t *counter) + SAFEALIGN_SET_VALUE(dest, value, uint16_t, pctr) + + #define SAFEALIGN_COPY_UINT32_CHECK(dest, src, len, pctr) do { \ +- if ((*(pctr) + sizeof(uint32_t)) > (len)) return EINVAL; \ ++ if ((*(pctr) + sizeof(uint32_t)) > (len) || \ ++ SIZE_T_OVERFLOW(*(pctr), sizeof(uint32_t))) return EINVAL; \ + safealign_memcpy(dest, src, sizeof(uint32_t), pctr); \ + } while(0) + + #define SAFEALIGN_COPY_INT32_CHECK(dest, src, len, pctr) do { \ +- if ((*(pctr) + sizeof(int32_t)) > (len)) return EINVAL; \ ++ if ((*(pctr) + sizeof(int32_t)) > (len) || \ ++ SIZE_T_OVERFLOW(*(pctr), sizeof(int32_t))) return EINVAL; \ + safealign_memcpy(dest, src, sizeof(int32_t), pctr); \ + } while(0) + +-- +1.7.3.2 + diff --git a/sssd.changes b/sssd.changes index 63a8a33..c0a8fb9 100644 --- a/sssd.changes +++ b/sssd.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Tue Jan 18 09:08:35 UTC 2011 - rhafer@suse.de + +- It was possible to make sssd hang forever inside a loop in the + PAM responder by sending a carefully crafted packet to sssd. + This could be exploited by a local attacker to crash sssd and + prevent other legitimate users from logging into the system. + (bnc#660481, CVE-2010-4341) + ------------------------------------------------------------------- Sun Dec 19 13:37:32 UTC 2010 - aj@suse.de diff --git a/sssd.spec b/sssd.spec index 565033b..81c211a 100644 --- a/sssd.spec +++ b/sssd.spec @@ -26,6 +26,8 @@ License: GPLv3+ and LGPLv3+ Url: https://fedorahosted.org/sssd/ Source0: %{name}-%{version}.tar.bz2 Source1: baselibs.conf +Patch0: 0001-Validate-user-supplied-size-of-data-items.patch +Patch1: 0002-Add-overflow-check-to-SAFEALIGN_COPY_-_CHECK-macros.patch BuildRoot: %{_tmppath}/%{name}-%{version}-build ### Dependencies ### @@ -103,6 +105,8 @@ Security Services Daemon (sssd). %prep %setup -q +%patch0 -p1 +%patch1 -p1 %build autoreconf