Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/httpnenv.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,17 @@ httpnenv(const UCHAR *name, const UCHAR *value)
size_t total = offsetof(HTTPV, name) + namelen + 1 + vallen + 1;
HTTPV *v;

if (namelen + vallen > 8192) return NULL; /* sanity limit */
/* Both rejects return NULL, so set errno to tell the caller which one it
was: httpsenv() reports an oversized variable differently from a storage
shortage, and since libc370#82 the shortage is a real outcome rather
than an S878 (issue #162). */
if (namelen + vallen > 8192) { /* sanity limit */
errno = E2BIG;
return NULL;
}

v = calloc(1, total);
if (!v) errno = ENOMEM;

if (v) {
strcpy(v->eye, HTTPV_EYE);
Expand Down
9 changes: 8 additions & 1 deletion src/httppc.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,14 @@ httppc(HTTPC *httpc)
char scriptfile[384];
snprintf(scriptfile, sizeof(scriptfile), "%s%s",
httpd->docroot, path);
http_set_env(httpc, "SCRIPT_FILENAME", scriptfile);
/* every other http_set_env() caller checks; dispatching the
CGI without SCRIPT_FILENAME would run it against no file at
all, so fail the request instead of guessing (#162) */
if (http_set_env(httpc, "SCRIPT_FILENAME", scriptfile)) {
http_resp_internal_error(httpc);
httpc->state = CSTATE_DONE;
goto check_done;
}
}

/* path needs to be processed by external program */
Expand Down
15 changes: 15 additions & 0 deletions src/httpsenv.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@ httpsenv(HTTPC *httpc, const UCHAR *name, const UCHAR *value)
/* allocate a new environment variable */
v = http_new_env(name, value);
if (!v) {
/* Say so. Before libc370#82 a storage shortage here abended S878 and
try(serve_client) logged HTTPD062E; now it is a quiet -1 that every
caller turns into CSTATE_RESET, i.e. a connection reset with no
response and no log line -- indistinguishable from a malformed
request (issue #162). This is the one choke point every env-var
caller passes through, and it still knows which variable failed,
which the reset sites no longer do. */
if (errno == E2BIG)
wtof("HTTPD905E Environment variable %.32s too large "
"client(%08X)", name, httpc);
else
wtof("HTTPD904E No storage for environment variable %.32s "
"client(%08X)", name, httpc);
rc = -1;
goto quit;
}
Expand All @@ -26,6 +39,8 @@ httpsenv(HTTPC *httpc, const UCHAR *name, const UCHAR *value)
the new HTTPV would be an unreachable orphan -- free it and report. */
if (array_add(&httpc->env, v)) {
free(v);
wtof("HTTPD904E No storage for environment variable %.32s "
"client(%08X)", name, httpc);
rc = -1;
}

Expand Down
17 changes: 12 additions & 5 deletions src/httpshen.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ parse_cookies(HTTPC *httpc, const UCHAR *in)
UCHAR *value;

if (!buf) {
wtof("%s: out of memory", __func__);
return ENOMEM;
wtof("HTTPD904E No storage for environment variable HTTP_Cookie "
"client(%08X)", httpc);
return -1; /* -1, like every other setter in this path */
}

for (name = buf; name && *name; ) {
/* skip leading delimiters */
while (*name == ';' || *name == ' ') name++;
Expand All @@ -49,10 +50,16 @@ parse_cookies(HTTPC *httpc, const UCHAR *in)
else {
value = "";
}
set_cookie(httpc, name, value);
/* stop at the first failure: the caller resets the connection
anyway, and carrying on would emit one HTTPD904E per remaining
cookie instead of one per request */
if (set_cookie(httpc, name, value)) {
free(buf);
return -1;
}
name = end;
}

free(buf);
return 0;
}
Expand Down
32 changes: 32 additions & 0 deletions test/tstnenv.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,37 @@ int main(void)
free(v);
}

/* Both rejects return NULL, and httpsenv() tells them apart by errno to
decide whether to report a storage shortage (HTTPD904E) or an oversized
variable (HTTPD905E) -- issue #162. The sanity limit is the half that
needs no storage shortage to reach, so it is the half a test can pin. */
{
UCHAR *big = malloc(9000);

CHECK(big != NULL, "oversize probe buffer allocated");
if (big) {
memset(big, 'X', 8999);
big[8999] = 0;

errno = 0;
v = http_new_env((const UCHAR *)"BIG", big);
CHECK(v == NULL, "name+value over the sanity limit is rejected");
CHECK(errno == E2BIG, "oversize reject reports E2BIG, not ENOMEM");
if (v) free(v);

/* just under the limit still succeeds -- guards the boundary
against the reject swallowing legitimate variables */
big[8000] = 0;
v = http_new_env((const UCHAR *)"BIG", big);
CHECK(v != NULL, "name+value under the sanity limit is accepted");
if (v) {
CHECK(v->value && strlen(v->value) == 8000,
"under-limit value stored at full length");
free(v);
}
free(big);
}
}

return mbt_test_summary("TSTNENV");
}