From: Philippe Antoine Date: Mon, 6 Dec 2021 08:22:52 +0000 (+0100) Subject: http: : fix int warnings X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=334b1382e0c647dfad71e9320350e87dbba90e1f;p=people%2Fms%2Fsuricata.git http: : fix int warnings Explicitly truncate file names to UINT16_MAX Before, they got implicitly truncated, meaning a UINT16_MAX + 1 file name, went to 0 file name (because of modulo 65536) --- diff --git a/rules/http-events.rules b/rules/http-events.rules index 90e08e7d3..dbdf523f5 100644 --- a/rules/http-events.rules +++ b/rules/http-events.rules @@ -85,4 +85,6 @@ alert http any any -> any any (msg:"SURICATA HTTP too many warnings"; flow:estab alert http any any -> any any (msg:"SURICATA HTTP invalid Range header value"; flow:established; app-layer-event:http.range_invalid; flowint:http.anomaly.count,+,1; classtype:protocol-command-decode; sid:2221051; rev:1;) -# next sid 2221052 +alert http any any -> any any (msg:"SURICATA HTTP file name too long"; flow:established; app-layer-event:http.file_name_too_long; flowint:http.anomaly.count,+,1; classtype:protocol-command-decode; sid:2221052; rev:1;) + +# next sid 2221053 diff --git a/src/app-layer-htp-file.c b/src/app-layer-htp-file.c index 4051d1811..83fbf0dab 100644 --- a/src/app-layer-htp-file.c +++ b/src/app-layer-htp-file.c @@ -227,8 +227,7 @@ int HTPFileOpenWithRange(HtpState *s, HtpTxUserData *txud, const uint8_t *filena HTTPContentRange crparsed; if (HTPParseAndCheckContentRange(rawvalue, &crparsed, s, htud) != 0) { // range is invalid, fall back to classic open - return HTPFileOpen( - s, txud, filename, (uint32_t)filename_len, data, data_len, txid, STREAM_TOCLIENT); + return HTPFileOpen(s, txud, filename, filename_len, data, data_len, txid, STREAM_TOCLIENT); } flags = FileFlowToFlags(s->f, STREAM_TOCLIENT); if ((s->flags & HTP_FLAG_STORE_FILES_TS) || diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c index 24777d3f9..ed7d6b154 100644 --- a/src/app-layer-htp-range.c +++ b/src/app-layer-htp-range.c @@ -163,7 +163,8 @@ void HttpRangeContainersInit(void) } } if (ConfGetValue("app-layer.protocols.http.byterange.timeout", &str) == 1) { - if (StringParseUint32(&timeout, 10, strlen(str), str) <= 0) { + size_t slen = strlen(str); + if (slen > UINT16_MAX || StringParseUint32(&timeout, 10, (uint16_t)slen, str) <= 0) { SCLogWarning(SC_ERR_INVALID_VALUE, "timeout value cannot be deduced: %s," " resetting to default", diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index bd169e10c..0d1916626 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -131,6 +131,7 @@ SCEnumCharMap http_decoder_event_table[] = { { "INVALID_RESPONSE_FIELD_FOLDING", HTTP_DECODER_EVENT_INVALID_RESPONSE_FIELD_FOLDING }, { "REQUEST_FIELD_TOO_LONG", HTTP_DECODER_EVENT_REQUEST_FIELD_TOO_LONG }, { "RESPONSE_FIELD_TOO_LONG", HTTP_DECODER_EVENT_RESPONSE_FIELD_TOO_LONG }, + { "FILE_NAME_TOO_LONG", HTTP_DECODER_EVENT_FILE_NAME_TOO_LONG }, { "REQUEST_LINE_INVALID", HTTP_DECODER_EVENT_REQUEST_LINE_INVALID }, { "REQUEST_BODY_UNEXPECTED", HTTP_DECODER_EVENT_REQUEST_BODY_UNEXPECTED }, { "REQUEST_SERVER_PORT_TCP_PORT_MISMATCH", @@ -524,7 +525,7 @@ static uint32_t AppLayerHtpComputeChunkLength(uint64_t content_len_so_far, uint3 /* below error messages updated up to libhtp 0.5.7 (git 379632278b38b9a792183694a4febb9e0dbd1e7a) */ struct { const char *msg; - int de; + uint8_t de; } htp_errors[] = { { "GZip decompressor: inflateInit2 failed", HTTP_DECODER_EVENT_GZIP_DECOMPRESSION_FAILED}, { "Request field invalid: colon missing", HTTP_DECODER_EVENT_REQUEST_FIELD_MISSING_COLON}, @@ -547,7 +548,7 @@ struct { struct { const char *msg; - int de; + uint8_t de; } htp_warnings[] = { { "GZip decompressor:", HTTP_DECODER_EVENT_GZIP_DECOMPRESSION_FAILED}, { "Request field invalid", HTTP_DECODER_EVENT_REQUEST_HEADER_INVALID}, @@ -594,7 +595,7 @@ struct { * * \retval id the id or 0 in case of not found */ -static int HTPHandleWarningGetId(const char *msg) +static uint8_t HTPHandleWarningGetId(const char *msg) { SCLogDebug("received warning \"%s\"", msg); size_t idx; @@ -618,7 +619,7 @@ static int HTPHandleWarningGetId(const char *msg) * * \retval id the id or 0 in case of not found */ -static int HTPHandleErrorGetId(const char *msg) +static uint8_t HTPHandleErrorGetId(const char *msg) { SCLogDebug("received error \"%s\"", msg); @@ -675,7 +676,7 @@ static void HTPHandleError(HtpState *s, const uint8_t dir) SCLogDebug("message %s", log->msg); - int id = HTPHandleErrorGetId(log->msg); + uint8_t id = HTPHandleErrorGetId(log->msg); if (id == 0) { id = HTPHandleWarningGetId(log->msg); if (id == 0) @@ -1254,9 +1255,9 @@ static void HtpRequestBodyMultipartParseHeader(HtpState *hstate, ft_len = USHRT_MAX; *filename = fn; - *filename_len = fn_len; + *filename_len = (uint16_t)fn_len; *filetype = ft; - *filetype_len = ft_len; + *filetype_len = (uint16_t)ft_len; } /** @@ -1303,8 +1304,8 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud, { int result = 0; uint8_t boundary[htud->boundary_len + 4]; /**< size limited to HTP_BOUNDARY_MAX + 4 */ - uint32_t expected_boundary_len = htud->boundary_len + 2; - uint32_t expected_boundary_end_len = htud->boundary_len + 4; + uint16_t expected_boundary_len = htud->boundary_len + 2; + uint16_t expected_boundary_end_len = htud->boundary_len + 4; int tx_progress = 0; #ifdef PRINT @@ -1433,7 +1434,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud, /* skip empty records */ if (expected_boundary_len == header_len) { goto next; - } else if ((expected_boundary_len + 2) <= header_len) { + } else if ((uint32_t)(expected_boundary_len + 2) <= header_len) { header_len -= (expected_boundary_len + 2); header = (uint8_t *)header_start + (expected_boundary_len + 2); // + for 0d 0a } @@ -1535,7 +1536,7 @@ static int HtpRequestBodyHandleMultipart(HtpState *hstate, HtpTxUserData *htud, SCLogDebug("offset %u", offset); htud->request_body.body_parsed += offset; - if (filedata_len >= (expected_boundary_len + 2)) { + if (filedata_len >= (uint32_t)(expected_boundary_len + 2)) { filedata_len -= (expected_boundary_len + 2 - 1); SCLogDebug("opening file with partial data"); } else { @@ -1629,7 +1630,12 @@ static int HtpRequestBodyHandlePOSTorPUT(HtpState *hstate, HtpTxUserData *htud, } if (filename != NULL) { - result = HTPFileOpen(hstate, htud, filename, (uint32_t)filename_len, data, data_len, + if (filename_len > PATH_MAX) { + // explicitly truncate the file name if too long + filename_len = PATH_MAX; + HTPSetEvent(hstate, htud, STREAM_TOSERVER, HTTP_DECODER_EVENT_FILE_NAME_TOO_LONG); + } + result = HTPFileOpen(hstate, htud, filename, (uint16_t)filename_len, data, data_len, HtpGetActiveRequestTxID(hstate), STREAM_TOSERVER); if (result == -1) { goto end; @@ -1702,11 +1708,16 @@ static int HtpResponseBodyHandle(HtpState *hstate, HtpTxUserData *htud, if (filename != NULL) { // set range if present htp_header_t *h_content_range = htp_table_get_c(tx->response_headers, "content-range"); + if (filename_len > PATH_MAX) { + // explicitly truncate the file name if too long + filename_len = PATH_MAX; + HTPSetEvent(hstate, htud, STREAM_TOSERVER, HTTP_DECODER_EVENT_FILE_NAME_TOO_LONG); + } if (h_content_range != NULL) { - result = HTPFileOpenWithRange(hstate, htud, filename, (uint32_t)filename_len, data, + result = HTPFileOpenWithRange(hstate, htud, filename, (uint16_t)filename_len, data, data_len, HtpGetActiveResponseTxID(hstate), h_content_range->value, htud); } else { - result = HTPFileOpen(hstate, htud, filename, (uint32_t)filename_len, data, data_len, + result = HTPFileOpen(hstate, htud, filename, (uint16_t)filename_len, data, data_len, HtpGetActiveResponseTxID(hstate), STREAM_TOCLIENT); } SCLogDebug("result %d", result); @@ -3028,7 +3039,7 @@ static int HTPRegisterPatternsForProtocolDetection(void) * but the pattern matching should only be one char */ register_result = AppLayerProtoDetectPMRegisterPatternCI(IPPROTO_TCP, ALPROTO_HTTP1, - method_buffer, strlen(method_buffer) - 3, 0, STREAM_TOSERVER); + method_buffer, (uint16_t)strlen(method_buffer) - 3, 0, STREAM_TOSERVER); if (register_result < 0) { return -1; } @@ -3038,7 +3049,8 @@ static int HTPRegisterPatternsForProtocolDetection(void) /* Loop through all the http verions patterns that are TO_CLIENT */ for (versions_pos = 0; versions[versions_pos]; versions_pos++) { register_result = AppLayerProtoDetectPMRegisterPatternCI(IPPROTO_TCP, ALPROTO_HTTP1, - versions[versions_pos], strlen(versions[versions_pos]), 0, STREAM_TOCLIENT); + versions[versions_pos], (uint16_t)strlen(versions[versions_pos]), 0, + STREAM_TOCLIENT); if (register_result < 0) { return -1; } diff --git a/src/app-layer-htp.h b/src/app-layer-htp.h index e23b08fc1..3a7001034 100644 --- a/src/app-layer-htp.h +++ b/src/app-layer-htp.h @@ -109,6 +109,7 @@ enum { HTTP_DECODER_EVENT_INVALID_RESPONSE_FIELD_FOLDING, HTTP_DECODER_EVENT_REQUEST_FIELD_TOO_LONG, HTTP_DECODER_EVENT_RESPONSE_FIELD_TOO_LONG, + HTTP_DECODER_EVENT_FILE_NAME_TOO_LONG, HTTP_DECODER_EVENT_REQUEST_SERVER_PORT_TCP_PORT_MISMATCH, HTTP_DECODER_EVENT_URI_HOST_INVALID, HTTP_DECODER_EVENT_HEADER_HOST_INVALID,