Fix case insensitive HTTP/MIME header name comparisons

Since the function was never used to check for anything other than
equality, I've changed it to return a bool, otherwise the changes to
its implementation are based on a patch submitted by Jeffrey Bencteux:

Function was comparing two strings based on the length of a user
provided string which could lead to evasions. Any prefix of the static
string could pass conditions where strcasecmp_n was used. Comparison
is now based on the static string length and lengths are checked
before calling strncasecmp.
This commit is contained in:
Jon Siwek 2018-05-08 14:01:04 -05:00
parent ec4a936f66
commit e5d5cf9ff1
3 changed files with 28 additions and 26 deletions

View file

@ -357,7 +357,7 @@ void HTTP_Entity::SetPlainDelivery(int64_t length)
void HTTP_Entity::SubmitHeader(mime::MIME_Header* h) void HTTP_Entity::SubmitHeader(mime::MIME_Header* h)
{ {
if ( mime::strcasecmp_n(h->get_name(), "content-length") == 0 ) if ( mime::istrequal(h->get_name(), "content-length") )
{ {
data_chunk_t vt = h->get_value_token(); data_chunk_t vt = h->get_value_token();
if ( ! mime::is_null_data_chunk(vt) ) if ( ! mime::is_null_data_chunk(vt) )
@ -383,7 +383,7 @@ void HTTP_Entity::SubmitHeader(mime::MIME_Header* h)
} }
// Figure out content-length for HTTP 206 Partial Content response // Figure out content-length for HTTP 206 Partial Content response
else if ( mime::strcasecmp_n(h->get_name(), "content-range") == 0 && else if ( mime::istrequal(h->get_name(), "content-range") &&
http_message->MyHTTP_Analyzer()->HTTP_ReplyCode() == 206 ) http_message->MyHTTP_Analyzer()->HTTP_ReplyCode() == 206 )
{ {
data_chunk_t vt = h->get_value_token(); data_chunk_t vt = h->get_value_token();
@ -468,7 +468,7 @@ void HTTP_Entity::SubmitHeader(mime::MIME_Header* h)
} }
} }
else if ( mime::strcasecmp_n(h->get_name(), "transfer-encoding") == 0 ) else if ( mime::istrequal(h->get_name(), "transfer-encoding") )
{ {
double http_version = 0; double http_version = 0;
if (http_message->analyzer->GetRequestOngoing()) if (http_message->analyzer->GetRequestOngoing())
@ -477,18 +477,16 @@ void HTTP_Entity::SubmitHeader(mime::MIME_Header* h)
http_version = http_message->analyzer->GetReplyVersion(); http_version = http_message->analyzer->GetReplyVersion();
data_chunk_t vt = h->get_value_token(); data_chunk_t vt = h->get_value_token();
if ( mime::strcasecmp_n(vt, "chunked") == 0 && if ( mime::istrequal(vt, "chunked") && http_version == 1.1 )
http_version == 1.1)
chunked_transfer_state = BEFORE_CHUNK; chunked_transfer_state = BEFORE_CHUNK;
} }
else if ( mime::strcasecmp_n(h->get_name(), "content-encoding") == 0 ) else if ( mime::istrequal(h->get_name(), "content-encoding") )
{ {
data_chunk_t vt = h->get_value_token(); data_chunk_t vt = h->get_value_token();
if ( mime::strcasecmp_n(vt, "gzip") == 0 || if ( mime::istrequal(vt, "gzip") || mime::istrequal(vt, "x-gzip") )
mime::strcasecmp_n(vt, "x-gzip") == 0 )
encoding = GZIP; encoding = GZIP;
if ( mime::strcasecmp_n(vt, "deflate") == 0 ) if ( mime::istrequal(vt, "deflate") )
encoding = DEFLATE; encoding = DEFLATE;
} }
@ -1651,11 +1649,11 @@ void HTTP_Analyzer::HTTP_Header(int is_orig, mime::MIME_Header* h)
{ {
#if 0 #if 0
// ### Only call ParseVersion if we're tracking versions: // ### Only call ParseVersion if we're tracking versions:
if ( strcasecmp_n(h->get_name(), "server") == 0 ) if ( istrequal(h->get_name(), "server") )
ParseVersion(h->get_value(), ParseVersion(h->get_value(),
(is_orig ? Conn()->OrigAddr() : Conn()->RespAddr()), false); (is_orig ? Conn()->OrigAddr() : Conn()->RespAddr()), false);
else if ( strcasecmp_n(h->get_name(), "user-agent") == 0 ) else if ( istrequal(h->get_name(), "user-agent") )
ParseVersion(h->get_value(), ParseVersion(h->get_value(),
(is_orig ? Conn()->OrigAddr() : Conn()->RespAddr()), true); (is_orig ? Conn()->OrigAddr() : Conn()->RespAddr()), true);
#endif #endif
@ -1664,23 +1662,23 @@ void HTTP_Analyzer::HTTP_Header(int is_orig, mime::MIME_Header* h)
// side, and if seen assume the connection to be persistent. // side, and if seen assume the connection to be persistent.
// This seems fairly safe - at worst, the client does indeed // This seems fairly safe - at worst, the client does indeed
// send additional requests, and the server ignores them. // send additional requests, and the server ignores them.
if ( is_orig && mime::strcasecmp_n(h->get_name(), "connection") == 0 ) if ( is_orig && mime::istrequal(h->get_name(), "connection") )
{ {
if ( mime::strcasecmp_n(h->get_value_token(), "keep-alive") == 0 ) if ( mime::istrequal(h->get_value_token(), "keep-alive") )
keep_alive = 1; keep_alive = 1;
} }
if ( ! is_orig && if ( ! is_orig &&
mime::strcasecmp_n(h->get_name(), "connection") == 0 ) mime::istrequal(h->get_name(), "connection") )
{ {
if ( mime::strcasecmp_n(h->get_value_token(), "close") == 0 ) if ( mime::istrequal(h->get_value_token(), "close") )
connection_close = 1; connection_close = 1;
else if ( mime::strcasecmp_n(h->get_value_token(), "upgrade") == 0 ) else if ( mime::istrequal(h->get_value_token(), "upgrade") )
upgrade_connection = true; upgrade_connection = true;
} }
if ( ! is_orig && if ( ! is_orig &&
mime::strcasecmp_n(h->get_name(), "upgrade") == 0 ) mime::istrequal(h->get_name(), "upgrade") )
upgrade_protocol.assign(h->get_value_token().data, h->get_value_token().length); upgrade_protocol.assign(h->get_value_token().data, h->get_value_token().length);
if ( http_header ) if ( http_header )

View file

@ -146,9 +146,14 @@ void MIME_Mail::Undelivered(int len)
is_orig, cur_entity_id); is_orig, cur_entity_id);
} }
int strcasecmp_n(data_chunk_t s, const char* t) bool istrequal(data_chunk_t s, const char* t)
{ {
return strncasecmp(s.data, t, s.length); int len = strlen(t);
if ( s.length != len )
return false;
return strncasecmp(s.data, t, len) == 0;
} }
int MIME_count_leading_lws(int len, const char* data) int MIME_count_leading_lws(int len, const char* data)
@ -751,7 +756,7 @@ int MIME_Entity::LookupMIMEHeaderName(data_chunk_t name)
// header names are case-insensitive (RFC 822, 2822, 2045). // header names are case-insensitive (RFC 822, 2822, 2045).
for ( int i = 0; MIMEHeaderName[i] != 0; ++i ) for ( int i = 0; MIMEHeaderName[i] != 0; ++i )
if ( strcasecmp_n(name, MIMEHeaderName[i]) == 0 ) if ( istrequal(name, MIMEHeaderName[i]) )
return i; return i;
return -1; return -1;
} }
@ -876,7 +881,7 @@ int MIME_Entity::ParseFieldParameters(int len, const char* data)
if ( current_field_type == MIME_CONTENT_TYPE && if ( current_field_type == MIME_CONTENT_TYPE &&
content_type == CONTENT_TYPE_MULTIPART && content_type == CONTENT_TYPE_MULTIPART &&
strcasecmp_n(attr, "boundary") == 0 ) istrequal(attr, "boundary") )
{ {
// token or quoted-string (and some lenience for characters // token or quoted-string (and some lenience for characters
// not explicitly allowed by the RFC, but encountered in the wild) // not explicitly allowed by the RFC, but encountered in the wild)
@ -915,13 +920,13 @@ void MIME_Entity::ParseContentType(data_chunk_t type, data_chunk_t sub_type)
{ {
int i; int i;
for ( i = 0; MIMEContentTypeName[i]; ++i ) for ( i = 0; MIMEContentTypeName[i]; ++i )
if ( strcasecmp_n(type, MIMEContentTypeName[i]) == 0 ) if ( istrequal(type, MIMEContentTypeName[i]) )
break; break;
content_type = i; content_type = i;
for ( i = 0; MIMEContentSubtypeName[i]; ++i ) for ( i = 0; MIMEContentSubtypeName[i]; ++i )
if ( strcasecmp_n(sub_type, MIMEContentSubtypeName[i]) == 0 ) if ( istrequal(sub_type, MIMEContentSubtypeName[i]) )
break; break;
content_subtype = i; content_subtype = i;
@ -942,8 +947,7 @@ void MIME_Entity::ParseContentEncoding(data_chunk_t encoding_mechanism)
{ {
int i; int i;
for ( i = 0; MIMEContentEncodingName[i]; ++i ) for ( i = 0; MIMEContentEncodingName[i]; ++i )
if ( strcasecmp_n(encoding_mechanism, if ( istrequal(encoding_mechanism, MIMEContentEncodingName[i]) )
MIMEContentEncodingName[i]) == 0 )
break; break;
content_encoding = i; content_encoding = i;

View file

@ -268,7 +268,7 @@ extern StringVal* new_string_val(int length, const char* data);
extern StringVal* new_string_val(const char* data, const char* end_of_data); extern StringVal* new_string_val(const char* data, const char* end_of_data);
extern StringVal* new_string_val(const data_chunk_t buf); extern StringVal* new_string_val(const data_chunk_t buf);
extern int fputs(data_chunk_t b, FILE* fp); extern int fputs(data_chunk_t b, FILE* fp);
extern int strcasecmp_n(data_chunk_t s, const char* t); extern bool istrequal(data_chunk_t s, const char* t);
extern int is_lws(char ch); extern int is_lws(char ch);
extern int MIME_is_field_name_char(char ch); extern int MIME_is_field_name_char(char ch);
extern int MIME_count_leading_lws(int len, const char* data); extern int MIME_count_leading_lws(int len, const char* data);