From 44692c83aa087b7a3bcbcaa7c8b93581354a1713 Mon Sep 17 00:00:00 2001 From: Gerardo Iglesias Galvan Date: Tue, 31 May 2011 20:10:05 -0500 Subject: [PATCH] Properly check retval for config and conversion function calls --- src/detect-fragoffset.c | 6 +++++- src/detect-icmp-id.c | 6 +++++- src/detect-icmp-seq.c | 6 +++++- src/detect-icode.c | 24 ++++++++++++++++++++---- src/detect-itype.c | 24 ++++++++++++++++++++---- src/detect-parse.c | 4 +++- src/util-classification-config.c | 6 ++++-- src/util-threshold-config.c | 6 ++++-- 8 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/detect-fragoffset.c b/src/detect-fragoffset.c index 22d18cef6e..513330c20b 100644 --- a/src/detect-fragoffset.c +++ b/src/detect-fragoffset.c @@ -179,7 +179,11 @@ DetectFragOffsetData *DetectFragOffsetParse (char *fragoffsetstr) { } } - ByteExtractStringUint16(&fragoff->frag_off, 10, 0, substr[1]); + if (ByteExtractStringUint16(&fragoff->frag_off, 10, 0, substr[1]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified frag offset %s is not " + "valid", substr[1]); + goto error; + } for (i = 0; i < 3; i++) { if (substr[i] != NULL) SCFree(substr[i]); diff --git a/src/detect-icmp-id.c b/src/detect-icmp-id.c index 8bc13ce95b..c93ff8568c 100644 --- a/src/detect-icmp-id.c +++ b/src/detect-icmp-id.c @@ -185,7 +185,11 @@ DetectIcmpIdData *DetectIcmpIdParse (char *icmpidstr) { /** \todo can ByteExtractStringUint16 do this? */ uint16_t id = 0; - ByteExtractStringUint16(&id, 10, 0, substr[1]); + if (ByteExtractStringUint16(&id, 10, 0, substr[1]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified icmp id %s is not " + "valid", substr[1]); + goto error; + } iid->id = htons(id); for (i = 0; i < 3; i++) { diff --git a/src/detect-icmp-seq.c b/src/detect-icmp-seq.c index 15a3c30283..9b6a9b0d00 100644 --- a/src/detect-icmp-seq.c +++ b/src/detect-icmp-seq.c @@ -185,7 +185,11 @@ DetectIcmpSeqData *DetectIcmpSeqParse (char *icmpseqstr) { } uint16_t seq = 0; - ByteExtractStringUint16(&seq, 10, 0, substr[1]); + if (ByteExtractStringUint16(&seq, 10, 0, substr[1]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified icmp seq %s is not " + "valid", substr[1]); + goto error; + } iseq->seq = htons(seq); for (i = 0; i < 3; i++) { diff --git a/src/detect-icode.c b/src/detect-icode.c index 7804351f15..693ce6eee0 100644 --- a/src/detect-icode.c +++ b/src/detect-icode.c @@ -174,15 +174,27 @@ DetectICodeData *DetectICodeParse(char *icodestr) { goto error; } /* we have only a comparison ("<", ">") */ - ByteExtractStringUint8(&icd->code1, 10, 0, args[1]); + if (ByteExtractStringUint8(&icd->code1, 10, 0, args[1]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified icmp code %s is not " + "valid", args[1]); + goto error; + } if ((strcmp(args[0], ">")) == 0) icd->mode = DETECT_ICODE_GT; else icd->mode = DETECT_ICODE_LT; } else { /* no "<", ">" */ /* we have a range ("<>") */ if (args[2] != NULL) { icd->mode = (uint8_t) DETECT_ICODE_RN; - ByteExtractStringUint8(&icd->code1, 10, 0, args[1]); - ByteExtractStringUint8(&icd->code2, 10, 0, args[2]); + if (ByteExtractStringUint8(&icd->code1, 10, 0, args[1]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified icmp code %s is not " + "valid", args[1]); + goto error; + } + if (ByteExtractStringUint8(&icd->code2, 10, 0, args[2]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified icmp code %s is not " + "valid", args[2]); + goto error; + } /* we check that the first given value in the range is less than the second, otherwise we swap them */ if (icd->code1 > icd->code2) { @@ -192,7 +204,11 @@ DetectICodeData *DetectICodeParse(char *icodestr) { } } else { /* we have an equality */ icd->mode = DETECT_ICODE_EQ; - ByteExtractStringUint8(&icd->code1, 10, 0, args[1]); + if (ByteExtractStringUint8(&icd->code1, 10, 0, args[1]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified icmp code %s is not " + "valid", args[1]); + goto error; + } } } diff --git a/src/detect-itype.c b/src/detect-itype.c index 494c8454d5..2c8fa3a481 100644 --- a/src/detect-itype.c +++ b/src/detect-itype.c @@ -174,15 +174,27 @@ DetectITypeData *DetectITypeParse(char *itypestr) { goto error; } /* we have only a comparison ("<", ">") */ - ByteExtractStringUint8(&itd->type1, 10, 0, args[1]); + if (ByteExtractStringUint8(&itd->type1, 10, 0, args[1]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified icmp type %s is not " + "valid", args[1]); + goto error; + } if ((strcmp(args[0], ">")) == 0) itd->mode = DETECT_ITYPE_GT; else itd->mode = DETECT_ITYPE_LT; } else { /* no "<", ">" */ /* we have a range ("<>") */ if (args[2] != NULL) { itd->mode = (uint8_t) DETECT_ITYPE_RN; - ByteExtractStringUint8(&itd->type1, 10, 0, args[1]); - ByteExtractStringUint8(&itd->type2, 10, 0, args[2]); + if (ByteExtractStringUint8(&itd->type1, 10, 0, args[1]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified icmp type %s is not " + "valid", args[1]); + goto error; + } + if (ByteExtractStringUint8(&itd->type2, 10, 0, args[2]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified icmp type %s is not " + "valid", args[2]); + goto error; + } /* we check that the first given value in the range is less than the second, otherwise we swap them */ if (itd->type1 > itd->type2) { @@ -192,7 +204,11 @@ DetectITypeData *DetectITypeParse(char *itypestr) { } } else { /* we have an equality */ itd->mode = DETECT_ITYPE_EQ; - ByteExtractStringUint8(&itd->type1, 10, 0, args[1]); + if (ByteExtractStringUint8(&itd->type1, 10, 0, args[1]) < 0) { + SCLogError(SC_ERR_INVALID_ARGUMENT, "specified icmp type %s is not " + "valid", args[1]); + goto error; + } } } diff --git a/src/detect-parse.c b/src/detect-parse.c index 01c9edc2f1..dbe37593d2 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -662,7 +662,9 @@ static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr) /* extract the substrings */ for (i = 1; i <= ret-1; i++) { - pcre_get_substring(optstr, ov, MAX_SUBSTRINGS, i, &arr[i-1]); + if (pcre_get_substring(optstr, ov, MAX_SUBSTRINGS, i, &arr[i-1]) < 0) { + goto error; + } //printf("SigParseOptions: arr[%" PRId32 "] = \"%s\"\n", i-1, arr[i-1]); } arr[i-1]=NULL; diff --git a/src/util-classification-config.c b/src/util-classification-config.c index 72bcae36f1..67d5db3029 100644 --- a/src/util-classification-config.c +++ b/src/util-classification-config.c @@ -140,9 +140,11 @@ int SCClassConfInitContext(DetectEngineCtx *de_ctx) */ static char *SCClassConfGetConfFilename(void) { - char *log_filename = (char *)default_file_path; + char *log_filename = NULL; - ConfGet("classification-file", &log_filename); + if (ConfGet("classification-file", &log_filename) != 1) { + log_filename = (char *)default_file_path; + } return log_filename; } diff --git a/src/util-threshold-config.c b/src/util-threshold-config.c index f141d73949..39be1b0ff2 100644 --- a/src/util-threshold-config.c +++ b/src/util-threshold-config.c @@ -68,9 +68,11 @@ static pcre_extra *rate_regex_study = NULL; */ char *SCThresholdConfGetConfFilename(void) { - char *log_filename = (char *)THRESHOLD_CONF_DEF_CONF_FILEPATH; + char *log_filename = NULL; - ConfGet("threshold-file", &log_filename); + if (ConfGet("threshold-file", &log_filename) != 1) { + log_filename = (char *)THRESHOLD_CONF_DEF_CONF_FILEPATH; + } return log_filename; }