]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
http: : fix int warnings
authorPhilippe Antoine <contact@catenacyber.fr>
Mon, 6 Dec 2021 08:22:52 +0000 (09:22 +0100)
committerVictor Julien <vjulien@oisf.net>
Thu, 13 Jan 2022 07:58:11 +0000 (08:58 +0100)
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)

rules/http-events.rules
src/app-layer-htp-file.c
src/app-layer-htp-range.c
src/app-layer-htp.c
src/app-layer-htp.h

index 90e08e7d3628d9a125db613f1742c1fe02d06ccd..dbdf523f52c444df1c1ecea7a25ff4308d8e61ad 100644 (file)
@@ -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
index 4051d1811c9a9c42109ebf4d7e2a8c0bdf3d58e7..83fbf0dab30defba5dc6ed0411fa919eaf68494c 100644 (file)
@@ -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) ||
index 24777d3f99b3df44fb0aa654d23c755aa4d83b96..ed7d6b1540592325dc32af4dadf3c85f359028be 100644 (file)
@@ -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",
index bd169e10c0ac2f168b88d0c16ec401b0bff2090a..0d19166260a1d7df8c0f55fe3beca1c9c689dacd 100644 (file)
@@ -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;
         }
index e23b08fc1d235beac7dee97a3ce022a58856f906..3a700103496ae1f9163c643dbb284881a5bae5a9 100644 (file)
@@ -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,