From: Victor Julien <vjulien@oisf.net>
Date: Thu, 25 Apr 2024 15:07:52 +0000 (+0200)
Subject: detect/iprep: allow 0 as a reputation value
X-Git-Tag: suricata-7.0.6~78
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=514f3c5d70dd3fca8eaabd72359575cc2713fc37;p=thirdparty%2Fsuricata.git

detect/iprep: allow 0 as a reputation value

Rules would allow checking against value 0, but internally the value
was used to indicate "no value". To address this, the internals now
return negative values for not found. This way value 0 can be fully
supported.

Bug: #6834.
(cherry picked from commit 64dc217f9f64b2e731757ce7bac40cf616e5166c)
---

diff --git a/src/detect-iprep.c b/src/detect-iprep.c
index 649ba24a2e..3d7ee1d5cd 100644
--- a/src/detect-iprep.c
+++ b/src/detect-iprep.c
@@ -75,65 +75,66 @@ void DetectIPRepRegister (void)
     sigmatch_table[DETECT_IPREP].flags |= SIGMATCH_IPONLY_COMPAT;
 }
 
-static inline uint8_t GetRep(const SReputation *r, const uint8_t cat, const uint32_t version)
+static inline int8_t GetRep(const SReputation *r, const uint8_t cat, const uint32_t version)
 {
     /* allow higher versions as this happens during
      * rule reload */
     if (r != NULL && r->version >= version) {
         return r->rep[cat];
     }
-    return 0;
+    return -1;
 }
 
-static uint8_t GetHostRepSrc(Packet *p, uint8_t cat, uint32_t version)
+/** \returns: -2 no host, -1 no rep entry, 0-127 rep values */
+static int8_t GetHostRepSrc(Packet *p, uint8_t cat, uint32_t version)
 {
     if (p->flags & PKT_HOST_SRC_LOOKED_UP && p->host_src == NULL) {
-        return 0;
+        return -2;
     } else if (p->host_src != NULL) {
         Host *h = (Host *)p->host_src;
         HostLock(h);
         /* use_cnt: 1 for having iprep, 1 for packet ref */
         DEBUG_VALIDATE_BUG_ON(h->iprep != NULL && SC_ATOMIC_GET(h->use_cnt) < 2);
-        uint8_t val = GetRep(h->iprep, cat, version);
+        int8_t val = GetRep(h->iprep, cat, version);
         HostUnlock(h);
         return val;
     } else {
         Host *h = HostLookupHostFromHash(&(p->src));
         p->flags |= PKT_HOST_SRC_LOOKED_UP;
         if (h == NULL)
-            return 0;
+            return -2;
         HostReference(&p->host_src, h);
         /* use_cnt: 1 for having iprep, 1 for HostLookupHostFromHash,
          * 1 for HostReference to packet */
         DEBUG_VALIDATE_BUG_ON(h->iprep != NULL && SC_ATOMIC_GET(h->use_cnt) < 3);
-        uint8_t val = GetRep(h->iprep, cat, version);
+        int8_t val = GetRep(h->iprep, cat, version);
         HostRelease(h); /* use_cnt >= 2: 1 for iprep, 1 for packet ref */
         return val;
     }
 }
 
-static uint8_t GetHostRepDst(Packet *p, uint8_t cat, uint32_t version)
+static int8_t GetHostRepDst(Packet *p, uint8_t cat, uint32_t version)
 {
     if (p->flags & PKT_HOST_DST_LOOKED_UP && p->host_dst == NULL) {
-        return 0;
+        return -2;
     } else if (p->host_dst != NULL) {
         Host *h = (Host *)p->host_dst;
         HostLock(h);
         /* use_cnt: 1 for having iprep, 1 for packet ref */
         DEBUG_VALIDATE_BUG_ON(h->iprep != NULL && SC_ATOMIC_GET(h->use_cnt) < 2);
-        uint8_t val = GetRep(h->iprep, cat, version);
+        int8_t val = GetRep(h->iprep, cat, version);
         HostUnlock(h);
         return val;
     } else {
         Host *h = HostLookupHostFromHash(&(p->dst));
         p->flags |= PKT_HOST_DST_LOOKED_UP;
         if (h == NULL)
-            return 0;
+            return -2;
         HostReference(&p->host_dst, h);
         /* use_cnt: 1 for having iprep, 1 for HostLookupHostFromHash,
          * 1 for HostReference to packet */
         DEBUG_VALIDATE_BUG_ON(h->iprep != NULL && SC_ATOMIC_GET(h->use_cnt) < 3);
-        uint8_t val = GetRep(h->iprep, cat, version);
+        int8_t val = GetRep(h->iprep, cat, version);
         HostRelease(h); /* use_cnt >= 2: 1 for iprep, 1 for packet ref */
         return val;
     }
@@ -152,58 +153,58 @@ static int DetectIPRepMatch (DetectEngineThreadCtx *det_ctx, Packet *p,
         return 0;
 
     uint32_t version = det_ctx->de_ctx->srep_version;
-    uint8_t val = 0;
+    int8_t val = 0;
 
     SCLogDebug("rd->cmd %u", rd->cmd);
     switch (rd->cmd) {
         case IPRepCmdAny:
             val = GetHostRepSrc(p, rd->cat, version);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepSrc(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val > 0) {
-                if (DetectU8Match(val, &rd->du8))
+            if (val >= 0) {
+                if (DetectU8Match((uint8_t)val, &rd->du8))
                     return 1;
             }
             val = GetHostRepDst(p, rd->cat, version);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepDst(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val > 0) {
-                return DetectU8Match(val, &rd->du8);
+            if (val >= 0) {
+                return DetectU8Match((uint8_t)val, &rd->du8);
             }
             break;
 
         case IPRepCmdSrc:
             val = GetHostRepSrc(p, rd->cat, version);
-            SCLogDebug("checking src -- val %u (looking for cat %u, val %u)", val, rd->cat,
+            SCLogDebug("checking src -- val %d (looking for cat %u, val %u)", val, rd->cat,
                     rd->du8.arg1);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepSrc(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val > 0) {
-                return DetectU8Match(val, &rd->du8);
+            if (val >= 0) {
+                return DetectU8Match((uint8_t)val, &rd->du8);
             }
             break;
 
         case IPRepCmdDst:
             SCLogDebug("checking dst");
             val = GetHostRepDst(p, rd->cat, version);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepDst(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val > 0) {
-                return DetectU8Match(val, &rd->du8);
+            if (val >= 0) {
+                return DetectU8Match((uint8_t)val, &rd->du8);
             }
             break;
 
         case IPRepCmdBoth:
             val = GetHostRepSrc(p, rd->cat, version);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepSrc(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val == 0 || DetectU8Match(val, &rd->du8) == 0)
+            if (val < 0 || DetectU8Match((uint8_t)val, &rd->du8) == 0)
                 return 0;
             val = GetHostRepDst(p, rd->cat, version);
-            if (val == 0)
+            if (val < 0)
                 val = SRepCIDRGetIPRepDst(det_ctx->de_ctx->srepCIDR_ctx, p, rd->cat, version);
-            if (val > 0) {
-                return DetectU8Match(val, &rd->du8);
+            if (val >= 0) {
+                return DetectU8Match((uint8_t)val, &rd->du8);
             }
             break;
     }
diff --git a/src/reputation.c b/src/reputation.c
index 75f3ba0c3f..cd51746490 100644
--- a/src/reputation.c
+++ b/src/reputation.c
@@ -124,31 +124,31 @@ static void SRepCIDRAddNetblock(SRepCIDRTree *cidr_ctx, char *ip, int cat, uint8
     }
 }
 
-static uint8_t SRepCIDRGetIPv4IPRep(SRepCIDRTree *cidr_ctx, uint8_t *ipv4_addr, uint8_t cat)
+static int8_t SRepCIDRGetIPv4IPRep(SRepCIDRTree *cidr_ctx, uint8_t *ipv4_addr, uint8_t cat)
 {
     void *user_data = NULL;
     (void)SCRadixFindKeyIPV4BestMatch(ipv4_addr, cidr_ctx->srepIPV4_tree[cat], &user_data);
     if (user_data == NULL)
-        return 0;
+        return -1;
 
     SReputation *r = (SReputation *)user_data;
     return r->rep[cat];
 }
 
-static uint8_t SRepCIDRGetIPv6IPRep(SRepCIDRTree *cidr_ctx, uint8_t *ipv6_addr, uint8_t cat)
+static int8_t SRepCIDRGetIPv6IPRep(SRepCIDRTree *cidr_ctx, uint8_t *ipv6_addr, uint8_t cat)
 {
     void *user_data = NULL;
     (void)SCRadixFindKeyIPV6BestMatch(ipv6_addr, cidr_ctx->srepIPV6_tree[cat], &user_data);
     if (user_data == NULL)
-        return 0;
+        return -1;
 
     SReputation *r = (SReputation *)user_data;
     return r->rep[cat];
 }
 
-uint8_t SRepCIDRGetIPRepSrc(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version)
+int8_t SRepCIDRGetIPRepSrc(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version)
 {
-    uint8_t rep = 0;
+    int8_t rep = -3;
 
     if (PKT_IS_IPV4(p))
         rep = SRepCIDRGetIPv4IPRep(cidr_ctx, (uint8_t *)GET_IPV4_SRC_ADDR_PTR(p), cat);
@@ -158,9 +158,9 @@ uint8_t SRepCIDRGetIPRepSrc(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint
     return rep;
 }
 
-uint8_t SRepCIDRGetIPRepDst(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version)
+int8_t SRepCIDRGetIPRepDst(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version)
 {
-    uint8_t rep = 0;
+    int8_t rep = -3;
 
     if (PKT_IS_IPV4(p))
         rep = SRepCIDRGetIPv4IPRep(cidr_ctx, (uint8_t *)GET_IPV4_DST_ADDR_PTR(p), cat);
diff --git a/src/reputation.h b/src/reputation.h
index 3ed94d9851..9f14adee65 100644
--- a/src/reputation.h
+++ b/src/reputation.h
@@ -49,8 +49,8 @@ void SRepDestroy(struct DetectEngineCtx_ *de_ctx);
 void SRepReloadComplete(void);
 int SRepHostTimedOut(Host *);
 
-uint8_t SRepCIDRGetIPRepSrc(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version);
-uint8_t SRepCIDRGetIPRepDst(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version);
+int8_t SRepCIDRGetIPRepSrc(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version);
+int8_t SRepCIDRGetIPRepDst(SRepCIDRTree *cidr_ctx, Packet *p, uint8_t cat, uint32_t version);
 void SRepResetVersion(void);
 int SRepLoadCatFileFromFD(FILE *fp);
 int SRepLoadFileFromFD(SRepCIDRTree *cidr_ctx, FILE *fp);