------------------------------------------------------------ revno: 11714 revision-id: squid3@treenet.co.nz-20121129111532-5nj8mwy7tfhsx1wz parent: squid3@treenet.co.nz-20121126102410-fo2w3p8mn0nnmewa committer: Amos Jeffries branch nick: 3.2 timestamp: Thu 2012-11-29 04:15:32 -0700 message: cachemgr.cgi: Memory Leaks and DoS Vulnerability * Ignore invalid Content-Length headers. * Limit received POST requests to 4KB and discard the rest. * Authentication credentials parser also leaks badly. Detected by Coverity Scan. Issues 740380, 740443, 740444, 740442, 740487, 740446, 740445 ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20121129111532-5nj8mwy7tfhsx1wz # target_branch: http://bzr.squid-cache.org/bzr/squid3/branches\ # /SQUID_3_2 # testament_sha1: 2db42382a72f71d402dc265e4da28399f2ca0409 # timestamp: 2012-11-29 11:21:54 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/branches\ # /SQUID_3_2 # base_revision_id: squid3@treenet.co.nz-20121126102410-\ # fo2w3p8mn0nnmewa # # Begin patch === modified file 'tools/cachemgr.cc' --- tools/cachemgr.cc 2012-07-28 05:38:50 +0000 +++ tools/cachemgr.cc 2012-11-29 11:15:32 +0000 @@ -596,12 +596,15 @@ if ((p = strchr(x, '\n'))) *p = '\0'; action = xstrtok(&x, '\t'); + if (!action) { + xfree(buf); + return ""; + } description = xstrtok(&x, '\t'); if (!description) description = action; - if (!action) - return ""; snprintf(html, sizeof(html), " %s", menu_url(req, action), description); + xfree(buf); return html; } @@ -830,7 +833,7 @@ } if (!check_target_acl(req->hostname, req->port)) { - snprintf(buf, 1024, "target %s:%d not allowed in cachemgr.conf\n", req->hostname, req->port); + snprintf(buf, sizeof(buf), "target %s:%d not allowed in cachemgr.conf\n", req->hostname, req->port); error_html(buf); return 1; } @@ -842,7 +845,7 @@ } else if ((S = req->hostname)) (void) 0; else { - snprintf(buf, 1024, "Unknown host: %s\n", req->hostname); + snprintf(buf, sizeof(buf), "Unknown host: %s\n", req->hostname); error_html(buf); return 1; } @@ -856,17 +859,19 @@ #else if ((s = socket(PF_INET, SOCK_STREAM, 0)) < 0) { #endif - snprintf(buf, 1024, "socket: %s\n", xstrerror()); + snprintf(buf, sizeof(buf), "socket: %s\n", xstrerror()); error_html(buf); + S.FreeAddrInfo(AI); return 1; } if (connect(s, AI->ai_addr, AI->ai_addrlen) < 0) { - snprintf(buf, 1024, "connect %s: %s\n", + snprintf(buf, sizeof(buf), "connect %s: %s\n", S.ToURL(ipbuf,MAX_IPSTRLEN), xstrerror()); error_html(buf); S.FreeAddrInfo(AI); + close(s); return 1; } @@ -954,8 +959,6 @@ read_post_request(void) { char *s; - char *buf; - int len; if ((s = getenv("REQUEST_METHOD")) == NULL) return NULL; @@ -966,15 +969,34 @@ if ((s = getenv("CONTENT_LENGTH")) == NULL) return NULL; - if ((len = atoi(s)) <= 0) - return NULL; - - buf = (char *)xmalloc(len + 1); - - if (fread(buf, len, 1, stdin) == 0) - return NULL; - - buf[len] = '\0'; + if (*s == '-') // negative length content huh? + return NULL; + + uint64_t len; + + char *endptr = s+ strlen(s); + if ((len = strtoll(s, &endptr, 10)) <= 0) + return NULL; + + // limit the input to something reasonable. + // 4KB should be enough for the GET/POST data length, but may be extended. + size_t bufLen = (len >= 4096 ? len : 4095); + char *buf = (char *)xmalloc(bufLen + 1); + + size_t readLen = fread(buf, bufLen, 1, stdin); + if (readLen == 0) { + xfree(buf); + return NULL; + } + buf[readLen] = '\0'; + len -= readLen; + + // purge the remainder of the request entity + while (len > 0) { + char temp[65535]; + readLen = fread(temp, 65535, 1, stdin); + len -= readLen; + } return buf; } @@ -1121,37 +1143,49 @@ debug("cmgr: length ok\n"); /* parse ( a lot of memory leaks, but that is cachemgr style :) */ - if ((host_name = strtok(buf, "|")) == NULL) + if ((host_name = strtok(buf, "|")) == NULL) { + xfree(buf); return; + } debug("cmgr: decoded host: '%s'\n", host_name); - if ((time_str = strtok(NULL, "|")) == NULL) + if ((time_str = strtok(NULL, "|")) == NULL) { + xfree(buf); return; + } debug("cmgr: decoded time: '%s' (now: %d)\n", time_str, (int) now); - if ((user_name = strtok(NULL, "|")) == NULL) + if ((user_name = strtok(NULL, "|")) == NULL) { + xfree(buf); return; + } debug("cmgr: decoded uname: '%s'\n", user_name); - if ((passwd = strtok(NULL, "|")) == NULL) + if ((passwd = strtok(NULL, "|")) == NULL) { + xfree(buf); return; + } debug("cmgr: decoded passwd: '%s'\n", passwd); /* verify freshness and validity */ - if (atoi(time_str) + passwd_ttl < now) + if (atoi(time_str) + passwd_ttl < now) { + xfree(buf); return; + } - if (strcasecmp(host_name, req->hostname)) + if (strcasecmp(host_name, req->hostname)) { + xfree(buf); return; + } debug("cmgr: verified auth. info.\n"); /* ok, accept */ - xfree(req->user_name); + safe_free(req->user_name); req->user_name = xstrdup(user_name); @@ -1193,6 +1227,7 @@ snprintf(&buf[stringLength], sizeof(buf) - stringLength, "Proxy-Authorization: Basic %s\r\n", str64); + xfree(str64); return buf; } ------------------------------------------------------------ revno: 11743 revision-id: squid3@treenet.co.nz-20130101052914-r4lk62270w4sh498 parent: squid3@treenet.co.nz-20121230074717-y9oekp0bfabt6szv committer: Amos Jeffries branch nick: 3.2 timestamp: Mon 2012-12-31 22:29:14 -0700 message: Additional pieces of SQUID-2012:1 ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20130101052914-r4lk62270w4sh498 # target_branch: http://bzr.squid-cache.org/bzr/squid3/branches\ # /SQUID_3_2 # testament_sha1: 3cf80543cf33a78ae27d5178a8e9958854350ca3 # timestamp: 2013-01-01 05:35:26 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/branches\ # /SQUID_3_2 # base_revision_id: squid3@treenet.co.nz-20121230074717-\ # y9oekp0bfabt6szv # # Begin patch === modified file 'tools/cachemgr.cc' --- tools/cachemgr.cc 2012-11-29 11:15:32 +0000 +++ tools/cachemgr.cc 2013-01-01 05:29:14 +0000 @@ -980,10 +980,10 @@ // limit the input to something reasonable. // 4KB should be enough for the GET/POST data length, but may be extended. - size_t bufLen = (len >= 4096 ? len : 4095); + size_t bufLen = (len < 4096 ? len : 4095); char *buf = (char *)xmalloc(bufLen + 1); - size_t readLen = fread(buf, bufLen, 1, stdin); + size_t readLen = fread(buf, 1, bufLen, stdin); if (readLen == 0) { xfree(buf); return NULL; @@ -994,7 +994,7 @@ // purge the remainder of the request entity while (len > 0) { char temp[65535]; - readLen = fread(temp, 65535, 1, stdin); + readLen = fread(temp, 1, 65535, stdin); len -= readLen; }