From bb2e11108b3ce5351f3abf45777c47fca37936af Mon Sep 17 00:00:00 2001
From: Victor Julien <vjulien@oisf.net>
Date: Tue, 30 Aug 2022 13:08:05 +0200
Subject: [PATCH] packetpool: fix uaf in debug validation check

Location of the check meant access to freed packet was possible.

Move check and simplify it to just look at the packet at hand.
---
 src/tmqh-packetpool.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/tmqh-packetpool.c b/src/tmqh-packetpool.c
index a07edd127d..e512aa114a 100644
--- a/src/tmqh-packetpool.c
+++ b/src/tmqh-packetpool.c
@@ -380,6 +380,14 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p)
     SCEnter();
     SCLogDebug("Packet %p, p->root %p, alloced %s", p, p->root, p->flags & PKT_ALLOC ? "true" : "false");
 
+#ifdef DEBUG_VALIDATION
+    /* Check that the drop reason has been set, if we have a drop.
+     * Use macro to avoid testing Packet::root. */
+    if (PACKET_TEST_ACTION(p, ACTION_DROP)) {
+        DEBUG_VALIDATE_BUG_ON((p)->drop_reason == PKT_DROP_REASON_NOT_SET);
+    }
+#endif
+
     if (IS_TUNNEL_PKT(p)) {
         SCLogDebug("Packet %p is a tunnel packet: %s",
             p,p->root ? "upper layer" : "tunnel root");
@@ -451,13 +459,6 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p)
         SCLogDebug("tunnel stuff done, move on (proot %d)", proot);
     }
 
-#ifdef DEBUG_VALIDATION
-    /* Check that the drop reason has been set, if we have a drop */
-    if (PacketTestAction(p, ACTION_DROP)) {
-        DEBUG_VALIDATE_BUG_ON((p)->drop_reason == PKT_DROP_REASON_NOT_SET);
-    }
-#endif
-
     /* we're done with the tunnel root now as well */
     if (proot == true) {
         SCLogDebug("getting rid of root pkt... alloc'd %s", p->root->flags & PKT_ALLOC ? "true" : "false");