From: Philippe Antoine <contact@catenacyber.fr>
Date: Mon, 16 Mar 2020 13:46:51 +0000 (+0100)
Subject: ftp: FTPParseResponse bufferizes lines
X-Git-Tag: suricata-5.0.3~47
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=75c7b3e0c2acf55686891c28935a68f01a7a3e9e;p=thirdparty%2Fsuricata.git

ftp: FTPParseResponse bufferizes lines

Protects against evasion by TCP packet splitting

The problem arised if the FTP response is split on multiple packets

The fix is to bufferize the content, until we get a complete line

(cherry picked from commit a6294d6ec25d0f2f0b5d25f7a824c7325e8f87ce)
---

diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c
index 9574cf98f9..ee17f24d46 100644
--- a/src/app-layer-ftp.c
+++ b/src/app-layer-ftp.c
@@ -735,7 +735,12 @@ static int FTPParseResponse(Flow *f, void *ftp_state, AppLayerParserState *pstat
     if (unlikely(input_len == 0)) {
         return 1;
     }
+    state->input = input;
+    state->input_len = input_len;
+    /* toclient stream */
+    state->direction = 1;
 
+    while (FTPGetLine(state) >= 0) {
     FTPTransaction *tx = FTPGetOldestTx(state);
     if (tx == NULL) {
         tx = FTPTransactionCreate(state);
@@ -750,7 +755,7 @@ static int FTPParseResponse(Flow *f, void *ftp_state, AppLayerParserState *pstat
 
     state->curr_tx = tx;
     if (state->command == FTP_COMMAND_AUTH_TLS) {
-        if (input_len >= 4 && SCMemcmp("234 ", input, 4) == 0) {
+        if (state->current_line_len >= 4 && SCMemcmp("234 ", state->current_line, 4) == 0) {
             AppLayerRequestProtocolTLSUpgrade(f);
         }
     }
@@ -784,32 +789,33 @@ static int FTPParseResponse(Flow *f, void *ftp_state, AppLayerParserState *pstat
     }
 
     if (state->command == FTP_COMMAND_PASV) {
-        if (input_len >= 4 && SCMemcmp("227 ", input, 4) == 0) {
-            FTPParsePassiveResponse(f, ftp_state, input, input_len);
+        if (state->current_line_len >= 4 && SCMemcmp("227 ", state->current_line, 4) == 0) {
+            FTPParsePassiveResponse(f, ftp_state, state->current_line, state->current_line_len);
         }
     }
 
     if (state->command == FTP_COMMAND_EPSV) {
-        if (input_len >= 4 && SCMemcmp("229 ", input, 4) == 0) {
-            FTPParsePassiveResponseV6(f, ftp_state, input, input_len);
+        if (state->current_line_len >= 4 && SCMemcmp("229 ", state->current_line, 4) == 0) {
+            FTPParsePassiveResponseV6(f, ftp_state, state->current_line, state->current_line_len);
         }
     }
 
-    if (likely(input_len)) {
+    if (likely(state->current_line_len)) {
         FTPString *response = FTPStringAlloc();
         if (likely(response)) {
-            response->len = CopyCommandLine(&response->str, input, input_len);
+            response->len = CopyCommandLine(&response->str, state->current_line, state->current_line_len);
             TAILQ_INSERT_TAIL(&tx->response_list, response, next);
         }
     }
 
     /* Handle preliminary replies -- keep tx open */
-    if (FTPIsPPR(input, input_len)) {
-        return retcode;
+    if (FTPIsPPR(state->current_line, state->current_line_len)) {
+        continue;
     }
-
 tx_complete:
     tx->done = true;
+    }
+
     return retcode;
 }