Fuzz tcpsplit v1 #4917

catenacyber · 2020-05-04T19:06:25Z

Link to redmine ticket:
None

Describe changes:

Adds a fuzz target with differential fuzzing against TCP splitting
Adds const keyword for some function arguments

Open questions :

Can we know if for some app layer protocol, it can be seen over TCP ? (how ?)
Code style

Tested with input file
alert ssh any any -> any any (msg:"SSH"; ssh.protoversion:2_compat; sid:1;)%0A%00%05%00%124%00%16SSH-2.0-test%0A

Ok with current version
Triggers abort with dummy patch

diff --git a/src/app-layer-ssh.c b/src/app-layer-ssh.c
index 910803f4a..662f7770e 100644
--- a/src/app-layer-ssh.c
+++ b/src/app-layer-ssh.c
@@ -343,7 +343,7 @@ static int EnoughData(const uint8_t *input, uint32_t input_len)
 {
     uint32_t u;
     for (u = 0; u < input_len; u++) {
-        if (input[u] == '\r' || input[u] == '\n')
+        if (input[u] == '\r' || input[u] == '\n' || input == '.')
             return TRUE;
     }
     return FALSE;

Tries tcp stream parsing against rule detection. And tries it again with splitting byte by byte. And checks if we get the same alerts.

catenacyber · 2020-05-04T19:07:18Z

src/detect-engine.c

@@ -2739,7 +2739,12 @@ TmEcode DetectEngineThreadCtxInit(ThreadVars *tv, void *initdata, void **data)
    memset(det_ctx, 0, sizeof(DetectEngineThreadCtx));

    det_ctx->tv = tv;
+#if defined(FUZZ)


inspired by unit tests mode a few lines below

catenacyber · 2020-05-04T19:08:03Z

src/suricata.c

@@ -2159,11 +2159,13 @@ static void SetupDelayedDetect(SCInstance *suri)

 static int LoadSignatures(DetectEngineCtx *de_ctx, SCInstance *suri)
 {
+#ifndef FUZZ
    if (SigLoadSignatures(de_ctx, suri->sig_file, suri->sig_file_exclusive) < 0) {


Not sure this is the best way

catenacyber · 2020-05-04T19:08:49Z

src/tests/fuzz/fuzz_tcpsplit.c

+TcpReassemblyThreadCtx *ra_ctx = NULL;
+uint64_t forceLayer = 0;
+
+static Packet *TestHelperBuildPacketAndFlow(const char *src, const char *dst,


Any good replacement ?

catenacyber · 2020-05-04T19:09:58Z

src/tests/fuzz/fuzz_tcpsplit.c

+}
+
+static int TestHelperAddSegmentWithPayload(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, TcpStream *stream, uint32_t seq, const uint8_t *payload, uint16_t len, Packet *p)
+{


inspired by StreamTcpUTAddSegmentWithPayload

The whole thing is inspired by tests such as DetectSshVersionTestDetect01 from ssh rust branch

catenacyber · 2020-05-04T19:11:19Z

src/tests/fuzz/fuzz_tcpsplit.c

+        goto end;
+    }
+    p->flow->protoctx = &ssn;
+    psplit->flow->protoctx = &ssnsplit;


TODO : have these in the helper function

catenacyber · 2020-05-04T19:13:31Z

This is a follow up to generalize bug findings such as #4792 or https://redmine.openinfosecfoundation.org/issues/3631

catenacyber · 2020-05-04T19:16:49Z

This will need corpus generation to serialized format from rule and pcap (and use S-V directories).

Are there rules who are meant to be sensitive to TCP splitting ?
Should we restrict this to app-layer rules ?

catenacyber added 2 commits May 4, 2020 21:01

check: Using const keyword for some arguments

e4cd5dd

fuzz: adds new differential fuzz target

5d90148

Tries tcp stream parsing against rule detection. And tries it again with splitting byte by byte. And checks if we get the same alerts.

catenacyber commented May 4, 2020

View reviewed changes

catenacyber closed this Sep 4, 2020

catenacyber deleted the fuzz-tcpsplit-v1 branch September 4, 2020 12:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fuzz tcpsplit v1 #4917

Fuzz tcpsplit v1 #4917

catenacyber commented May 4, 2020

catenacyber May 4, 2020

catenacyber May 4, 2020

catenacyber May 4, 2020

catenacyber May 4, 2020

catenacyber May 4, 2020

catenacyber commented May 4, 2020 •

edited

catenacyber commented May 4, 2020

Fuzz tcpsplit v1 #4917

Fuzz tcpsplit v1 #4917

Conversation

catenacyber commented May 4, 2020

catenacyber May 4, 2020

Choose a reason for hiding this comment

catenacyber May 4, 2020

Choose a reason for hiding this comment

catenacyber May 4, 2020

Choose a reason for hiding this comment

catenacyber May 4, 2020

Choose a reason for hiding this comment

catenacyber May 4, 2020

Choose a reason for hiding this comment

catenacyber commented May 4, 2020 • edited

catenacyber commented May 4, 2020

catenacyber commented May 4, 2020 •

edited