From 4ed72addf381a3c205811967a9cf17df056d9289 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Sun, 22 Dec 2019 11:21:17 +0530 Subject: [PATCH] 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. Partially closes redmine ticket 3053. --- src/detect-lua.c | 30 +++++++++++++++++++++++++--- src/detect-nfs-procedure.c | 13 ++++++++---- src/detect-nfs-version.c | 14 ++++++++----- src/detect-stream_size.c | 7 +++++-- src/detect-tcpmss.c | 34 ++++++++++++++++++++++--------- src/detect-template.c | 12 ++++++++--- src/detect-template2.c | 40 ++++++++++++++++++++++++++++--------- src/detect-ttl.c | 41 ++++++++++++++++++++++++++++---------- 8 files changed, 146 insertions(+), 45 deletions(-) diff --git a/src/detect-lua.c b/src/detect-lua.c index 93b071a476..2db5e68c94 100644 --- a/src/detect-lua.c +++ b/src/detect-lua.c @@ -43,6 +43,7 @@ #include "util-debug.h" #include "util-spm-bm.h" #include "util-print.h" +#include "util-byte.h" #include "util-unittest.h" #include "util-unittest-helper.h" @@ -280,8 +281,15 @@ int DetectLuaMatchBuffer(DetectEngineThreadCtx *det_ctx, SCLogDebug("k='%s', v='%s'", k, v); if (strcmp(k, "retval") == 0) { - if (atoi(v) == 1) + int val; + if (StringParseInt32(&val, 10, 0, (const char *)v) < 0) { + SCLogError(SC_ERR_INVALID_VALUE, "Invalid value " + "for \"retval\" from LUA return table: '%s'", v); + ret = 0; + } + else if (val == 1) { ret = 1; + } } else { /* set flow var? */ } @@ -428,8 +436,16 @@ static int DetectLuaMatch (DetectEngineThreadCtx *det_ctx, SCLogDebug("k='%s', v='%s'", k, v); if (strcmp(k, "retval") == 0) { - if (atoi(v) == 1) + int val; + if (StringParseInt32(&val, 10, 0, + (const char *)v) < 0) { + SCLogError(SC_ERR_INVALID_VALUE, "Invalid value " + "for \"retval\" from LUA return table: '%s'", v); + ret = 0; + } + else if (val == 1) { ret = 1; + } } else { /* set flow var? */ } @@ -530,8 +546,16 @@ static int DetectLuaAppMatchCommon (DetectEngineThreadCtx *det_ctx, SCLogDebug("k='%s', v='%s'", k, v); if (strcmp(k, "retval") == 0) { - if (atoi(v) == 1) + int val; + if (StringParseInt32(&val, 10, 0, + (const char *)v) < 0) { + SCLogError(SC_ERR_INVALID_VALUE, "Invalid value " + "for \"retval\" from LUA return table: '%s'", v); + ret = 0; + } + else if (val == 1) { ret = 1; + } } else { /* set flow var? */ } diff --git a/src/detect-nfs-procedure.c b/src/detect-nfs-procedure.c index 0778318c33..81c5476b13 100644 --- a/src/detect-nfs-procedure.c +++ b/src/detect-nfs-procedure.c @@ -42,6 +42,7 @@ #include "util-unittest.h" #include "util-unittest-helper.h" +#include "util-byte.h" #include "app-layer-nfs-tcp.h" #include "rust.h" @@ -287,8 +288,10 @@ static DetectNfsProcedureData *DetectNfsProcedureParse (const char *rawstr) } /* set the first value */ - dd->lo = atoi(value1); //TODO - + if (StringParseUint32(&dd->lo, 10, 0, (const char *)value1) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "Invalid first value: \"%s\"", value1); + goto error; + } /* set the second value if specified */ if (strlen(value2) > 0) { if (!(dd->mode == PROCEDURE_RA)) { @@ -297,8 +300,10 @@ static DetectNfsProcedureData *DetectNfsProcedureParse (const char *rawstr) goto error; } - // - dd->hi = atoi(value2); // TODO + if (StringParseUint32(&dd->hi, 10, 0, (const char *)value2) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "Invalid second value: \"%s\"", value2); + goto error; + } if (dd->hi <= dd->lo) { SCLogError(SC_ERR_INVALID_ARGUMENT, diff --git a/src/detect-nfs-version.c b/src/detect-nfs-version.c index 95cac98d8c..570cbabd7e 100644 --- a/src/detect-nfs-version.c +++ b/src/detect-nfs-version.c @@ -42,6 +42,7 @@ #include "util-unittest.h" #include "util-unittest-helper.h" +#include "util-byte.h" #include "app-layer-nfs-tcp.h" #include "rust.h" @@ -278,8 +279,10 @@ static DetectNfsVersionData *DetectNfsVersionParse (const char *rawstr) } /* set the first value */ - dd->lo = atoi(value1); //TODO - + if (StringParseUint32(&dd->lo, 10, 0, (const char *)value1) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "Invalid first value: \"%s\"", value1); + goto error; + } /* set the second value if specified */ if (strlen(value2) > 0) { if (!(dd->mode == PROCEDURE_RA)) { @@ -288,9 +291,10 @@ static DetectNfsVersionData *DetectNfsVersionParse (const char *rawstr) goto error; } - // - dd->hi = atoi(value2); // TODO - + if (StringParseUint32(&dd->hi, 10, 0, (const char *)value2) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "Invalid second value: \"%s\"", value2); + goto error; + } if (dd->hi <= dd->lo) { SCLogError(SC_ERR_INVALID_ARGUMENT, "Second value in range must not be smaller than the first"); diff --git a/src/detect-stream_size.c b/src/detect-stream_size.c index 476a3aca19..3d4d6e7861 100644 --- a/src/detect-stream_size.c +++ b/src/detect-stream_size.c @@ -34,6 +34,7 @@ #include "detect-stream_size.h" #include "stream-tcp-private.h" #include "util-debug.h" +#include "util-byte.h" /** * \brief Regex for parsing our flow options @@ -244,8 +245,10 @@ static DetectStreamSizeData *DetectStreamSizeParse (DetectEngineCtx *de_ctx, con } /* set the value */ - sd->ssize = (uint32_t)atoi(value); - + if (StringParseUint32(&sd->ssize, 10, 0, (const char *)value) < 0) { + SCLogError(SC_ERR_INVALID_VALUE, "Invalid value for stream size: %s", value); + goto error; + } /* inspect our options and set the flags */ if (strcmp(arg, "server") == 0) { sd->flags |= STREAM_SIZE_SERVER; diff --git a/src/detect-tcpmss.c b/src/detect-tcpmss.c index 6e28dca05f..ede86e7ac9 100644 --- a/src/detect-tcpmss.c +++ b/src/detect-tcpmss.c @@ -27,6 +27,7 @@ #include "detect.h" #include "detect-parse.h" #include "detect-engine-prefilter-common.h" +#include "util-byte.h" #include "detect-tcpmss.h" @@ -178,8 +179,10 @@ static DetectTcpmssData *DetectTcpmssParse (const char *tcpmssstr) goto error; tcpmssd->mode = DETECT_TCPMSS_LT; - tcpmssd->arg1 = (uint16_t) atoi(arg3); - + if (StringParseUint16(&tcpmssd->arg1, 10, 0, (const char *)arg3) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first arg: %s", arg3); + goto error; + } SCLogDebug("tcpmss is %"PRIu16"",tcpmssd->arg1); if (strlen(arg1) > 0) goto error; @@ -190,8 +193,10 @@ static DetectTcpmssData *DetectTcpmssParse (const char *tcpmssstr) goto error; tcpmssd->mode = DETECT_TCPMSS_GT; - tcpmssd->arg1 = (uint16_t) atoi(arg3); - + if (StringParseUint16(&tcpmssd->arg1, 10, 0, (const char *)arg3) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first arg: %s", arg3); + goto error; + } SCLogDebug("tcpmss is %"PRIu16"",tcpmssd->arg1); if (strlen(arg1) > 0) goto error; @@ -204,9 +209,14 @@ static DetectTcpmssData *DetectTcpmssParse (const char *tcpmssstr) goto error; tcpmssd->mode = DETECT_TCPMSS_RA; - tcpmssd->arg1 = (uint16_t) atoi(arg1); - - tcpmssd->arg2 = (uint16_t) atoi(arg3); + if (StringParseUint16(&tcpmssd->arg1, 10, 0, (const char *)arg1) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first arg: %s", arg1); + goto error; + } + if (StringParseUint16(&tcpmssd->arg2, 10, 0, (const char *)arg3) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid second arg: %s", arg3); + goto error; + } SCLogDebug("tcpmss is %"PRIu16" to %"PRIu16"",tcpmssd->arg1, tcpmssd->arg2); if (tcpmssd->arg1 >= tcpmssd->arg2) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid tcpmss range. "); @@ -221,7 +231,10 @@ static DetectTcpmssData *DetectTcpmssParse (const char *tcpmssstr) (arg1 == NULL ||strlen(arg1) == 0)) goto error; - tcpmssd->arg1 = (uint16_t) atoi(arg1); + if (StringParseUint16(&tcpmssd->arg1, 10, 0, (const char *)arg1) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first arg: %s", arg1); + goto error; + } break; } } else { @@ -231,7 +244,10 @@ static DetectTcpmssData *DetectTcpmssParse (const char *tcpmssstr) (arg1 == NULL ||strlen(arg1) == 0)) goto error; - tcpmssd->arg1 = (uint16_t) atoi(arg1); + if (StringParseUint16(&tcpmssd->arg1, 10, 0, (const char *)arg1) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first arg: %s", arg1); + goto error; + } } SCFree(arg1); diff --git a/src/detect-template.c b/src/detect-template.c index f16431d6d7..3fd1f5f32c 100644 --- a/src/detect-template.c +++ b/src/detect-template.c @@ -25,6 +25,7 @@ #include "suricata-common.h" #include "util-unittest.h" +#include "util-byte.h" #include "detect-parse.h" #include "detect-engine.h" @@ -154,9 +155,14 @@ static DetectTemplateData *DetectTemplateParse (const char *templatestr) if (unlikely(templated == NULL)) return NULL; - templated->arg1 = (uint8_t)atoi(arg1); - templated->arg2 = (uint8_t)atoi(arg2); - + if (ByteExtractStringUint8(&templated->arg1, 10, 0, (const char *)arg1) < 0) { + SCFree(templated); + return NULL; + } + if (ByteExtractStringUint8(&templated->arg2, 10, 0, (const char *)arg2) < 0) { + SCFree(templated); + return NULL; + } return templated; } diff --git a/src/detect-template2.c b/src/detect-template2.c index eef205a569..ee7ca5d0ff 100644 --- a/src/detect-template2.c +++ b/src/detect-template2.c @@ -23,6 +23,7 @@ */ #include "suricata-common.h" +#include "util-byte.h" #include "detect.h" #include "detect-parse.h" @@ -185,8 +186,11 @@ static DetectTemplate2Data *DetectTemplate2Parse (const char *template2str) goto error; template2d->mode = DETECT_TEMPLATE2_LT; - template2d->arg1 = (uint8_t) atoi(arg3); - + if (StringParseUint8(&template2d->arg1, 10, 0, (const char *)arg3) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first arg:" + " \"%s\"", arg3); + goto error; + } SCLogDebug("template2 is %"PRIu8"",template2d->arg1); if (strlen(arg1) > 0) goto error; @@ -197,8 +201,11 @@ static DetectTemplate2Data *DetectTemplate2Parse (const char *template2str) goto error; template2d->mode = DETECT_TEMPLATE2_GT; - template2d->arg1 = (uint8_t) atoi(arg3); - + if (StringParseUint8(&template2d->arg1, 10, 0, (const char *)arg3) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first arg:" + " \"%s\"", arg3); + goto error; + } SCLogDebug("template2 is %"PRIu8"",template2d->arg1); if (strlen(arg1) > 0) goto error; @@ -211,9 +218,16 @@ static DetectTemplate2Data *DetectTemplate2Parse (const char *template2str) goto error; template2d->mode = DETECT_TEMPLATE2_RA; - template2d->arg1 = (uint8_t) atoi(arg1); - - template2d->arg2 = (uint8_t) atoi(arg3); + if (StringParseUint8(&template2d->arg1, 10, 0, (const char *)arg1) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first arg:" + " \"%s\"", arg1); + goto error; + } + if (StringParseUint8(&template2d->arg2, 10, 0, (const char *)arg3) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid second arg:" + " \"%s\"", arg3); + goto error; + } SCLogDebug("template2 is %"PRIu8" to %"PRIu8"",template2d->arg1, template2d->arg2); if (template2d->arg1 >= template2d->arg2) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid template2 range. "); @@ -228,7 +242,11 @@ static DetectTemplate2Data *DetectTemplate2Parse (const char *template2str) (arg1 == NULL ||strlen(arg1) == 0)) goto error; - template2d->arg1 = (uint8_t) atoi(arg1); + if (StringParseUint8(&template2d->arg1, 10, 0, (const char *)arg1) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first arg:" + " \"%s\"", arg1); + goto error; + } break; } } else { @@ -238,7 +256,11 @@ static DetectTemplate2Data *DetectTemplate2Parse (const char *template2str) (arg1 == NULL ||strlen(arg1) == 0)) goto error; - template2d->arg1 = (uint8_t) atoi(arg1); + if (StringParseUint8(&template2d->arg1, 10, 0, (const char *)arg1) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first arg:" + " \"%s\"", arg1); + goto error; + } } SCFree(arg1); diff --git a/src/detect-ttl.c b/src/detect-ttl.c index 488abc77a7..dbc093d19d 100644 --- a/src/detect-ttl.c +++ b/src/detect-ttl.c @@ -33,6 +33,7 @@ #include "detect-ttl.h" #include "util-debug.h" +#include "util-byte.h" /** * \brief Regex for parsing our ttl options @@ -179,8 +180,11 @@ static DetectTtlData *DetectTtlParse (const char *ttlstr) return NULL; mode = DETECT_TTL_LT; - ttl1 = atoi(arg3); - + if (StringParseInt32(&ttl1, 10, 0, (const char *)arg3) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first ttl " + "value: \"%s\"", arg3); + return NULL; + } SCLogDebug("ttl is %d",ttl1); if (strlen(arg1) > 0) return NULL; @@ -191,8 +195,11 @@ static DetectTtlData *DetectTtlParse (const char *ttlstr) return NULL; mode = DETECT_TTL_GT; - ttl1 = atoi(arg3); - + if (StringParseInt32(&ttl1, 10, 0, (const char *)arg3) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first ttl " + "value: \"%s\"", arg3); + return NULL; + } SCLogDebug("ttl is %d",ttl1); if (strlen(arg1) > 0) return NULL; @@ -203,9 +210,17 @@ static DetectTtlData *DetectTtlParse (const char *ttlstr) return NULL; mode = DETECT_TTL_RA; - ttl1 = atoi(arg1); - ttl2 = atoi(arg3); + if (StringParseInt32(&ttl1, 10, 0, (const char *)arg1) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first ttl " + "value: \"%s\"", arg1); + return NULL; + } + if (StringParseInt32(&ttl2, 10, 0, (const char *)arg3) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid second ttl " + "value: \"%s\"", arg3); + return NULL; + } SCLogDebug("ttl is %d to %d",ttl1, ttl2); if (ttl1 >= ttl2) { SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid ttl range"); @@ -219,8 +234,11 @@ static DetectTtlData *DetectTtlParse (const char *ttlstr) (strlen(arg3) > 0) || (strlen(arg1) == 0)) return NULL; - - ttl1 = atoi(arg1); + if (StringParseInt32(&ttl1, 10, 0, (const char *)arg1) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first ttl " + "value: \"%s\"", arg1); + return NULL; + } break; } } else { @@ -229,8 +247,11 @@ static DetectTtlData *DetectTtlParse (const char *ttlstr) if ((strlen(arg3) > 0) || (strlen(arg1) == 0)) return NULL; - - ttl1 = atoi(arg1); + if (StringParseInt32(&ttl1, 10, 0, (const char *)arg1) < 0) { + SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid first ttl " + "value: \"%s\"", arg1); + return NULL; + } } if (ttl1 < 0 || ttl1 > UCHAR_MAX ||