From e5d5cf9ff13477344d1a34f531b1b5a2fc5b4df5 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 8 May 2018 14:01:04 -0500 Subject: [PATCH] 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. --- src/analyzer/protocol/http/HTTP.cc | 32 ++++++++++++++---------------- src/analyzer/protocol/mime/MIME.cc | 20 +++++++++++-------- src/analyzer/protocol/mime/MIME.h | 2 +- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/analyzer/protocol/http/HTTP.cc b/src/analyzer/protocol/http/HTTP.cc index 4cba28ec34..4e1e5a2218 100644 --- a/src/analyzer/protocol/http/HTTP.cc +++ b/src/analyzer/protocol/http/HTTP.cc @@ -357,7 +357,7 @@ void HTTP_Entity::SetPlainDelivery(int64_t length) 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(); 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 - 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 ) { 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; if (http_message->analyzer->GetRequestOngoing()) @@ -477,18 +477,16 @@ void HTTP_Entity::SubmitHeader(mime::MIME_Header* h) http_version = http_message->analyzer->GetReplyVersion(); data_chunk_t vt = h->get_value_token(); - if ( mime::strcasecmp_n(vt, "chunked") == 0 && - http_version == 1.1) + if ( mime::istrequal(vt, "chunked") && http_version == 1.1 ) 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(); - if ( mime::strcasecmp_n(vt, "gzip") == 0 || - mime::strcasecmp_n(vt, "x-gzip") == 0 ) + if ( mime::istrequal(vt, "gzip") || mime::istrequal(vt, "x-gzip") ) encoding = GZIP; - if ( mime::strcasecmp_n(vt, "deflate") == 0 ) + if ( mime::istrequal(vt, "deflate") ) encoding = DEFLATE; } @@ -1651,11 +1649,11 @@ void HTTP_Analyzer::HTTP_Header(int is_orig, mime::MIME_Header* h) { #if 0 // ### 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(), (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(), (is_orig ? Conn()->OrigAddr() : Conn()->RespAddr()), true); #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. // This seems fairly safe - at worst, the client does indeed // 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; } 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; - else if ( mime::strcasecmp_n(h->get_value_token(), "upgrade") == 0 ) + else if ( mime::istrequal(h->get_value_token(), "upgrade") ) upgrade_connection = true; } 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); if ( http_header ) diff --git a/src/analyzer/protocol/mime/MIME.cc b/src/analyzer/protocol/mime/MIME.cc index bcdfe03248..19d3dbe5d3 100644 --- a/src/analyzer/protocol/mime/MIME.cc +++ b/src/analyzer/protocol/mime/MIME.cc @@ -146,9 +146,14 @@ void MIME_Mail::Undelivered(int len) 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) @@ -751,7 +756,7 @@ int MIME_Entity::LookupMIMEHeaderName(data_chunk_t name) // header names are case-insensitive (RFC 822, 2822, 2045). for ( int i = 0; MIMEHeaderName[i] != 0; ++i ) - if ( strcasecmp_n(name, MIMEHeaderName[i]) == 0 ) + if ( istrequal(name, MIMEHeaderName[i]) ) return i; return -1; } @@ -876,7 +881,7 @@ int MIME_Entity::ParseFieldParameters(int len, const char* data) if ( current_field_type == MIME_CONTENT_TYPE && content_type == CONTENT_TYPE_MULTIPART && - strcasecmp_n(attr, "boundary") == 0 ) + istrequal(attr, "boundary") ) { // token or quoted-string (and some lenience for characters // 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; for ( i = 0; MIMEContentTypeName[i]; ++i ) - if ( strcasecmp_n(type, MIMEContentTypeName[i]) == 0 ) + if ( istrequal(type, MIMEContentTypeName[i]) ) break; content_type = i; for ( i = 0; MIMEContentSubtypeName[i]; ++i ) - if ( strcasecmp_n(sub_type, MIMEContentSubtypeName[i]) == 0 ) + if ( istrequal(sub_type, MIMEContentSubtypeName[i]) ) break; content_subtype = i; @@ -942,8 +947,7 @@ void MIME_Entity::ParseContentEncoding(data_chunk_t encoding_mechanism) { int i; for ( i = 0; MIMEContentEncodingName[i]; ++i ) - if ( strcasecmp_n(encoding_mechanism, - MIMEContentEncodingName[i]) == 0 ) + if ( istrequal(encoding_mechanism, MIMEContentEncodingName[i]) ) break; content_encoding = i; diff --git a/src/analyzer/protocol/mime/MIME.h b/src/analyzer/protocol/mime/MIME.h index 2ee58f24d1..a9ef89b932 100644 --- a/src/analyzer/protocol/mime/MIME.h +++ b/src/analyzer/protocol/mime/MIME.h @@ -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 data_chunk_t buf); 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 MIME_is_field_name_char(char ch); extern int MIME_count_leading_lws(int len, const char* data);