src: remove multiple uses of atoi

atoi() and related functions lack a mechanism for reporting errors for
invalid values. Replace them with calls to the appropriate
ByteExtractString* functions.

Closes redmine ticket 3053.

diff --git a/src/detect-byte-extract.c b/src/detect-byte-extract.c
index 8305c226b51df8e1b7d65634511486453d4c8754..cdcf7f24fa40e9efa20c14bae04224d0e0cc6638 100644 (file)
--- a/src/detect-byte-extract.c
+++ b/src/detect-byte-extract.c
@@ -240,7 +240,12 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
                    "for arg 1 for byte_extract");
         goto error;
     }
-    bed->nbytes = atoi(nbytes_str);
+    if (StringParseUint8(&bed->nbytes, 10, 0,
+                               (const char *)nbytes_str) < 0) {
+        SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid value for number of bytes"
+                   " to be extracted: \"%s\".", nbytes_str);
+        goto error;
+    }
 
     /* offset */
     char offset_str[64] = "";
@@ -251,10 +256,9 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
                    "for arg 2 for byte_extract");
         goto error;
     }
-    int offset = atoi(offset_str);
-    if (offset < -65535 || offset > 65535) {
-        SCLogError(SC_ERR_INVALID_SIGNATURE, "byte_extract offset invalid - %d.  "
-                   "The right offset range is -65535 to 65535", offset);
+    int32_t offset;
+    if (StringParseI32RangeCheck(&offset, 10, 0, (const char *)offset_str, -65535, 65535) < 0) {
+        SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid value for offset: \"%s\".", offset_str);
         goto error;
     }
     bed->offset = offset;
@@ -307,14 +311,13 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
                            "for arg %d for byte_extract", i);
                 goto error;
             }
-            int multiplier = atoi(multiplier_str);
-            if (multiplier < DETECT_BYTE_EXTRACT_MULTIPLIER_MIN_LIMIT ||
-                multiplier > DETECT_BYTE_EXTRACT_MULTIPLIER_MAX_LIMIT) {
-                SCLogError(SC_ERR_INVALID_SIGNATURE, "multipiler_value invalid "
-                           "- %d.  The range is %d-%d",
-                           multiplier,
-                           DETECT_BYTE_EXTRACT_MULTIPLIER_MIN_LIMIT,
-                           DETECT_BYTE_EXTRACT_MULTIPLIER_MAX_LIMIT);
+            int32_t multiplier;
+            if (StringParseI32RangeCheck(&multiplier, 10, 0,
+                                 (const char *)multiplier_str,
+                                 DETECT_BYTE_EXTRACT_MULTIPLIER_MIN_LIMIT,
+                                 DETECT_BYTE_EXTRACT_MULTIPLIER_MAX_LIMIT) < 0) {
+                SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid value for"
+                        "multiplier: \"%s\".", multiplier_str);
                 goto error;
             }
             bed->multiplier_value = multiplier;
@@ -411,7 +414,12 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
                            "for arg %d in byte_extract", i);
                 goto error;
             }
-            bed->align_value = atoi(align_str);
+            if (StringParseUint8(&bed->align_value, 10, 0,
+                                       (const char *)align_str) < 0) {
+                SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid align_value: "
+                           "\"%s\".", align_str);
+                goto error;
+            }
             if (!(bed->align_value == 2 || bed->align_value == 4)) {
                 SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid align_value for "
                            "byte_extract - \"%d\"", bed->align_value);

diff --git a/src/detect-cipservice.c b/src/detect-cipservice.c
index 712a7d8ae31514ccd58257fbfdfddeb9dc6f17bf..765a8a76d2920ac520be6b10f0a0ff63cb4546f6 100644 (file)
--- a/src/detect-cipservice.c
+++ b/src/detect-cipservice.c
@@ -346,14 +346,14 @@ static DetectEnipCommandData *DetectEnipCommandParse(const char *rulestr)
         goto error;
     }
 
-    unsigned long cmd = atol(rulestr);
-    if (cmd > MAX_ENIP_CMD) //if command greater than 16 bit
-    {
-        SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid ENIP command %lu", cmd);
+    uint16_t cmd;
+    if (StringParseUint16(&cmd, 10, 0, rulestr) < 0) {
+        SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid ENIP command"
+                   ": \"%s\"", rulestr);
         goto error;
     }
 
-    enipcmdd->enipcommand = (uint16_t) atoi(rulestr);
+    enipcmdd->enipcommand = cmd;
 
     return enipcmdd;
 

diff --git a/src/detect-engine-port.c b/src/detect-engine-port.c
index f0e768b78b6b9ba7919cd5034fb73eedeacb8574..88fb5b23cd875e51a495787d66e18336388425fc 100644 (file)
--- a/src/detect-engine-port.c
+++ b/src/detect-engine-port.c
@@ -51,12 +51,13 @@
 #include "host.h"
 #include "util-profiling.h"
 #include "util-var.h"
+#include "util-byte.h"
 
 static int DetectPortCutNot(DetectPort *, DetectPort **);
 static int DetectPortCut(DetectEngineCtx *, DetectPort *, DetectPort *,
                          DetectPort **);
 DetectPort *PortParse(const char *str);
-int DetectPortIsValidRange(char *);
+static bool DetectPortIsValidRange(char *, uint16_t *);
 
 /**
  * \brief Alloc a DetectPort structure and update counters
@@ -1295,20 +1296,20 @@ DetectPort *PortParse(const char *str)
     }
 
     /* see if the address is an ipv4 or ipv6 address */
-    if ((port2 = strchr(port, ':')) != NULL)  {
+    if ((port2 = strchr(port, ':')) != NULL) {
         /* 80:81 range format */
         port2[0] = '\0';
         port2++;
 
-        if (DetectPortIsValidRange(port))
-            dp->port = atoi(port);
-        else
-            goto error;
+        if (strcmp(port, "") != 0) {
+            if (!DetectPortIsValidRange(port, &dp->port))
+                goto error;
+        } else {
+            dp->port = 0;
+        }
 
         if (strcmp(port2, "") != 0) {
-            if (DetectPortIsValidRange(port2))
-                dp->port2 = atoi(port2);
-            else
+            if (!DetectPortIsValidRange(port2, &dp->port2))
                 goto error;
         } else {
             dp->port2 = 65535;
@@ -1321,10 +1322,10 @@ DetectPort *PortParse(const char *str)
         if (strcasecmp(port,"any") == 0) {
             dp->port = 0;
             dp->port2 = 65535;
-        } else if(DetectPortIsValidRange(port)){
-            dp->port = dp->port2 = atoi(port);
         } else {
-            goto error;
+            if (!DetectPortIsValidRange(port, &dp->port))
+                goto error;
+            dp->port2 = dp->port;
         }
     }
 
@@ -1342,18 +1343,16 @@ error:
  *
  * \param str Pointer to the port string
  *
- * \retval 1 if port is in the valid range
- * \retval 0 if invalid
+ *
+ * \retval true if port is in the valid range
+ * \retval false if invalid
  */
-int DetectPortIsValidRange(char *port)
+static bool DetectPortIsValidRange(char *port, uint16_t *port_val)
 {
-    char *end;
-    long r = strtol(port, &end, 10);
+    if (StringParseUint16(port_val, 10, 0, (const char *)port) < 0)
+        return false;
 
-    if(*end == 0 && r >= 0 && r <= 65535)
-        return 1;
-    else
-        return 0;
+    return true;
 }
 
 /********************** End parsing routines ********************/

diff --git a/src/detect-fast-pattern.c b/src/detect-fast-pattern.c
index 96286c0fec826d22c12fd51cb8c1e4e9948f5411..b7c60078bc19cea9d9c74fda7167d37f9ff9e758 100644 (file)
--- a/src/detect-fast-pattern.c
+++ b/src/detect-fast-pattern.c
@@ -33,6 +33,7 @@
 #include "detect-fast-pattern.h"
 
 #include "util-error.h"
+#include "util-byte.h"
 #include "util-debug.h"
 #include "util-unittest.h"
 #include "util-unittest-helper.h"
@@ -283,10 +284,11 @@ static int DetectFastPatternSetup(DetectEngineCtx *de_ctx, Signature *s, const c
                        "for fast_pattern offset");
             goto error;
         }
-        int offset = atoi(arg_substr);
-        if (offset > 65535) {
-            SCLogError(SC_ERR_INVALID_SIGNATURE, "Fast pattern offset exceeds "
-                       "limit");
+        uint16_t offset;
+        if (StringParseUint16(&offset, 10, 0,
+                              (const char *)arg_substr) < 0) {
+            SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid fast pattern offset:"
+                       " \"%s\"", arg_substr);
             goto error;
         }
 
@@ -297,14 +299,16 @@ static int DetectFastPatternSetup(DetectEngineCtx *de_ctx, Signature *s, const c
                        "for fast_pattern offset");
             goto error;
         }
-        int length = atoi(arg_substr);
-        if (length > 65535) {
-            SCLogError(SC_ERR_INVALID_SIGNATURE, "Fast pattern length exceeds "
-                       "limit");
+        uint16_t length;
+        if (StringParseUint16(&length, 10, 0,
+                              (const char *)arg_substr) < 0) {
+            SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid value for fast "
+                       "pattern: \"%s\"", arg_substr);
             goto error;
         }
 
-        if (offset + length > 65535) {
+        // Avoiding integer overflow
+        if (offset > (65535 - length)) {
             SCLogError(SC_ERR_INVALID_SIGNATURE, "Fast pattern (length + offset) "
                        "exceeds limit pattern length limit");
             goto error;

diff --git a/src/detect-ipproto.c b/src/detect-ipproto.c
index ea0373e67fa157a2c20c6935d85cff0351f2b0b6..023aa02c845c458ed2c59b6bf505bca41afce661 100644 (file)
--- a/src/detect-ipproto.c
+++ b/src/detect-ipproto.c
@@ -478,7 +478,8 @@ static int DetectIPProtoTestParse02(void)
 static int DetectIPProtoTestSetup01(void)
 {
     const char *value_str = "14";
-    int value = atoi(value_str);
+    int value;
+    FAIL_IF(StringParseInt32(&value, 10, 0, (const char *)value_str) < 0);
     int i;
 
     Signature *sig = SigAlloc();

diff --git a/src/detect-iprep.c b/src/detect-iprep.c
index 27616c5be0d85d5a3989aaa3d27d598d31213b55..e418fb2fd637e92716ae6ddb214ca4b40d72c70a 100644 (file)
--- a/src/detect-iprep.c
+++ b/src/detect-iprep.c
@@ -41,6 +41,7 @@
 #include "detect-engine-state.h"
 
 #include "util-debug.h"
+#include "util-byte.h"
 #include "util-unittest.h"
 #include "util-unittest-helper.h"
 #include "util-fmemopen.h"
@@ -321,10 +322,8 @@ int DetectIPRepSetup (DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
     }
 
     if (value != NULL && strlen(value) > 0) {
-        int ival = atoi(value);
-        if (ival < 0 || ival > 127)
+        if (StringParseU8RangeCheck(&val, 10, 0, (const char *)value, 0, 127) < 0)
             goto error;
-        val = (uint8_t)ival;
     }
 
     cd = SCMalloc(sizeof(DetectIPRepData));

diff --git a/src/detect-krb5-errcode.c b/src/detect-krb5-errcode.c
index 16bd2571524b5d2f082f924142b285c03156330f..1b7de6f214cdd0186ca90e4c61dcbae15dd97240 100644 (file)
--- a/src/detect-krb5-errcode.c
+++ b/src/detect-krb5-errcode.c
@@ -23,6 +23,7 @@
 
 #include "suricata-common.h"
 #include "util-unittest.h"
+#include "util-byte.h"
 
 #include "detect-parse.h"
 #include "detect-engine.h"
@@ -156,8 +157,10 @@ static DetectKrb5ErrCodeData *DetectKrb5ErrCodeParse (const char *krb5str)
     krb5d = SCMalloc(sizeof (DetectKrb5ErrCodeData));
     if (unlikely(krb5d == NULL))
         goto error;
-    krb5d->err_code = (int32_t)atoi(arg1);
-
+    if (StringParseInt32(&krb5d->err_code, 10, 0,
+                         (const char *)arg1) < 0) {
+        goto error;
+    }
     return krb5d;
 
 error:

diff --git a/src/detect-krb5-msgtype.c b/src/detect-krb5-msgtype.c
index e10ad3abca51844988201f0a7110eef078e14688..a7b71a829a6aa0de6941826cfa8a614e05d8c09c 100644 (file)
--- a/src/detect-krb5-msgtype.c
+++ b/src/detect-krb5-msgtype.c
@@ -23,6 +23,7 @@
 
 #include "suricata-common.h"
 #include "util-unittest.h"
+#include "util-byte.h"
 
 #include "detect-parse.h"
 #include "detect-engine.h"
@@ -153,8 +154,10 @@ static DetectKrb5MsgTypeData *DetectKrb5MsgTypeParse (const char *krb5str)
     krb5d = SCMalloc(sizeof (DetectKrb5MsgTypeData));
     if (unlikely(krb5d == NULL))
         goto error;
-    krb5d->msg_type = (uint8_t)atoi(arg1);
-
+    if (StringParseUint8(&krb5d->msg_type, 10, 0,
+                         (const char *)arg1) < 0) {
+        goto error;
+    }
     return krb5d;
 
 error:

diff --git a/src/detect-modbus.c b/src/detect-modbus.c
index f9beb4126a3ed8330099eb3d8bdc9c3a012cd0d6..24075cd8f3a542215b641971ef9b54853e2a7d89 100644 (file)
--- a/src/detect-modbus.c
+++ b/src/detect-modbus.c
@@ -51,6 +51,7 @@
 #include "detect-engine-modbus.h"
 
 #include "util-debug.h"
+#include "util-byte.h"
 
 #include "app-layer-modbus.h"
 
@@ -190,14 +191,21 @@ static DetectModbus *DetectModbusAccessParse(DetectEngineCtx *de_ctx, const char
             if (unlikely(modbus->address == NULL))
                 goto error;
 
+            uint8_t idx;
             if (arg[0] == '>') {
-                modbus->address->min    = atoi((const char*) (arg+1));
+                idx = 1;
                 modbus->address->mode   = DETECT_MODBUS_GT;
             } else if (arg[0] == '<') {
-                modbus->address->min    = atoi((const char*) (arg+1));
+                idx = 1;
                 modbus->address->mode   = DETECT_MODBUS_LT;
             } else {
-                modbus->address->min    = atoi((const char*) arg);
+                idx = 0;
+            }
+            if (StringParseUint16(&modbus->address->min, 10, 0,
+                                  (const char *) (arg + idx)) < 0) {
+                SCLogError(SC_ERR_INVALID_VALUE, "Invalid value for min "
+                           "address: %s", (const char *)(arg + idx));
+                goto error;
             }
             SCLogDebug("and min/equal address %d", modbus->address->min);
 
@@ -209,7 +217,12 @@ static DetectModbus *DetectModbusAccessParse(DetectEngineCtx *de_ctx, const char
                 }
 
                 if (*arg != '\0') {
-                    modbus->address->max    = atoi((const char*) (arg+2));
+                    if (StringParseUint16(&modbus->address->max, 10, 0,
+                                         (const char*) (arg + 2)) < 0) {
+                        SCLogError(SC_ERR_INVALID_VALUE, "Invalid value for max "
+                                   "address: %s", (const char*)(arg + 2));
+                        goto error;
+                    }
                     modbus->address->mode   = DETECT_MODBUS_RA;
                     SCLogDebug("and max address %d", modbus->address->max);
                 }
@@ -235,14 +248,22 @@ static DetectModbus *DetectModbusAccessParse(DetectEngineCtx *de_ctx, const char
                     if (unlikely(modbus->data == NULL))
                         goto error;
 
+
+                    uint8_t idx_mode;
                     if (arg[0] == '>') {
-                        modbus->data->min   = atoi((const char*) (arg+1));
+                        idx_mode = 1;
                         modbus->data->mode  = DETECT_MODBUS_GT;
                     } else if (arg[0] == '<') {
-                        modbus->data->min   = atoi((const char*) (arg+1));
+                        idx_mode = 1;
                         modbus->data->mode  = DETECT_MODBUS_LT;
                     } else {
-                        modbus->data->min   = atoi((const char*) arg);
+                        idx_mode = 0;
+                    }
+                    if (StringParseUint16(&modbus->data->min, 10, 0,
+                                          (const char*) (arg + idx_mode)) < 0) {
+                        SCLogError(SC_ERR_INVALID_VALUE, "Invalid value for "
+                                   "min data: %s", (const char*)(arg + idx_mode));
+                        goto error;
                     }
                     SCLogDebug("and min/equal value %d", modbus->data->min);
 
@@ -254,7 +275,13 @@ static DetectModbus *DetectModbusAccessParse(DetectEngineCtx *de_ctx, const char
                         }
 
                         if (*arg != '\0') {
-                            modbus->data->max   = atoi((const char*) (arg+2));
+                            if (StringParseUint16(&modbus->data->max,
+                                                  10, 0, (const char*) (arg + 2)) < 0) {
+                                SCLogError(SC_ERR_INVALID_VALUE,
+                                           "Invalid value for max data: %s",
+                                           (const char*)(arg + 2));
+                                goto error;
+                            }
                             modbus->data->mode  = DETECT_MODBUS_RA;
                             SCLogDebug("and max value %d", modbus->data->max);
                         }
@@ -306,7 +333,11 @@ static DetectModbus *DetectModbusFunctionParse(DetectEngineCtx *de_ctx, const ch
         goto error;
 
     if (isdigit((unsigned char)ptr[0])) {
-        modbus->function = atoi((const char*) ptr);
+        if (StringParseUint8(&modbus->function, 10, 0, (const char *)ptr) < 0) {
+            SCLogError(SC_ERR_INVALID_VALUE, "Invalid value for "
+                       "modbus function: %s", (const char *)ptr);
+            goto error;
+        }
         /* Function code 0 is managed by decoder_event INVALID_FUNCTION_CODE */
         if (modbus->function == MODBUS_FUNC_NONE) {
             SCLogError(SC_ERR_INVALID_SIGNATURE,
@@ -329,7 +360,12 @@ static DetectModbus *DetectModbusFunctionParse(DetectEngineCtx *de_ctx, const ch
             if (modbus->subfunction == NULL)
                 goto error;
 
-            *(modbus->subfunction) = atoi((const char*) arg);
+            if (StringParseUint16(&(*modbus->subfunction), 10, 0, (const char *)arg) < 0) {
+                SCLogError(SC_ERR_INVALID_VALUE, "Invalid value for "
+                           "modbus subfunction: %s", (const char*)arg);
+                goto error;
+            }
+
             SCLogDebug("and subfunction %d", *(modbus->subfunction));
         }
     } else {
@@ -421,14 +457,20 @@ static DetectModbus *DetectModbusUnitIdParse(DetectEngineCtx *de_ctx, const char
     if (unlikely(modbus->unit_id == NULL))
         goto error;
 
+    uint8_t idx;
     if (arg[0] == '>') {
-        modbus->unit_id->min   = atoi((const char*) (arg+1));
+        idx = 1;
         modbus->unit_id->mode  = DETECT_MODBUS_GT;
     } else if (arg[0] == '<') {
-        modbus->unit_id->min   = atoi((const char*) (arg+1));
+        idx = 1;
         modbus->unit_id->mode  = DETECT_MODBUS_LT;
     } else {
-        modbus->unit_id->min   = atoi((const char*) arg);
+        idx = 0;
+    }
+    if (StringParseUint16(&modbus->unit_id->min, 10, 0, (const char *) (arg + idx)) < 0) {
+        SCLogError(SC_ERR_INVALID_VALUE, "Invalid value for "
+                   "modbus min unit id: %s", (const char*)(arg + idx));
+        goto error;
     }
     SCLogDebug("and min/equal unit id %d", modbus->unit_id->min);
 
@@ -440,7 +482,11 @@ static DetectModbus *DetectModbusUnitIdParse(DetectEngineCtx *de_ctx, const char
         }
 
         if (*arg != '\0') {
-            modbus->unit_id->max   = atoi((const char*) (arg+2));
+            if (StringParseUint16(&modbus->unit_id->max, 10, 0, (const char *) (arg + 2)) < 0) {
+                SCLogError(SC_ERR_INVALID_VALUE, "Invalid value for "
+                           "modbus max unit id: %s", (const char*)(arg + 2));
+                goto error;
+            }
             modbus->unit_id->mode  = DETECT_MODBUS_RA;
             SCLogDebug("and max unit id %d", modbus->unit_id->max);
         }