Prevent size_t overflow in evhttp_htmlescape.

Modified the `html_replace' function so that it returns the length of
the replacement string instead of the string itself. This is used to
easily check for overflows of the `new_size' variable in the first for
loop of the `evhttp_htmlescape' function, and thus potential out of
bounds writes in the second for loop (if an overflow occurs in
new_size, then new_size < old_size). Also check that new_size + 1
doesn't overflow in mm_malloc(new_size + 1).

Removed the `scratch_space' variable from the `evhttp_htmlescape'
function since it wasn't actually used; also removed the `buf'
variable from the `evhttp_htmlescape' function since it was only used
by `scratch_space'.
This commit is contained in:
Mansour Moufid 2011-05-23 18:01:24 -04:00 committed by Nick Mathewson
parent 74760f1864
commit 06c51cdf93

52
http.c
View File

@ -219,29 +219,30 @@ strsep(char **s, const char *del)
} }
#endif #endif
static const char * static size_t
html_replace(char ch, char *buf) html_replace(const char ch, const char **escaped)
{ {
switch (ch) { switch (ch) {
case '<': case '<':
return "&lt;"; *escaped = "&lt;";
return 4;
case '>': case '>':
return "&gt;"; *escaped = "&gt;";
return 4;
case '"': case '"':
return "&quot;"; *escaped = "&quot;";
return 6;
case '\'': case '\'':
return "&#039;"; *escaped = "&#039;";
return 6;
case '&': case '&':
return "&amp;"; *escaped = "&amp;";
return 5;
default: default:
break; break;
} }
/* Echo the character back */ return 1;
buf[0] = ch;
buf[1] = '\0';
return buf;
} }
/* /*
@ -255,21 +256,34 @@ char *
evhttp_htmlescape(const char *html) evhttp_htmlescape(const char *html)
{ {
size_t i; size_t i;
size_t new_size = 0, old_size = strlen(html); size_t new_size = 0, old_size = 0;
char *escaped_html, *p; char *escaped_html, *p;
char scratch_space[2];
for (i = 0; i < old_size; ++i) if (html == NULL)
new_size += strlen(html_replace(html[i], scratch_space)); return (NULL);
old_size = strlen(html);
for (i = 0; i < old_size; ++i) {
const char *replaced = NULL;
const size_t replace_size = html_replace(html[i], &replaced);
if (replace_size > EV_SIZE_MAX - new_size) {
event_warn("%s: html_replace overflow", __func__);
return (NULL);
}
new_size += replace_size;
}
if (new_size == EV_SIZE_MAX)
return (NULL);
p = escaped_html = mm_malloc(new_size + 1); p = escaped_html = mm_malloc(new_size + 1);
if (escaped_html == NULL) { if (escaped_html == NULL) {
event_warn("%s: malloc(%ld)", __func__, (long)(new_size + 1)); event_warn("%s: malloc(%lu)", __func__,
(unsigned long)(new_size + 1));
return (NULL); return (NULL);
} }
for (i = 0; i < old_size; ++i) { for (i = 0; i < old_size; ++i) {
const char *replaced = html_replace(html[i], scratch_space); const char *replaced = &html[i];
size_t len = strlen(replaced); const size_t len = html_replace(html[i], &replaced);
memcpy(p, replaced, len); memcpy(p, replaced, len);
p += len; p += len;
} }