From cf375cf362a6556f292f9f474d28667f77e7c057 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Mon, 28 Nov 2022 11:09:50 +0100 Subject: [PATCH] ftp: Harden reply handing a bit and don't raise bad replies to script-land This improves runtime of the oss-fuzz generated traffic in #125. Specifically, that reproducers included a 064- reply code that was interpreted as needing to be continued. Also, return after AnalyzerViolations() for server replies rather than propagating bad replies them to script-land. This trusts server's to generally behave according to specification. --- src/analyzer/protocol/ftp/FTP.cc | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/analyzer/protocol/ftp/FTP.cc b/src/analyzer/protocol/ftp/FTP.cc index 914c6c2c8b..f0dcdeb5a4 100644 --- a/src/analyzer/protocol/ftp/FTP.cc +++ b/src/analyzer/protocol/ftp/FTP.cc @@ -166,25 +166,39 @@ void FTP_Analyzer::DeliverStream(int length, const u_char* data, bool orig) } else { // a new reply - if ( reply_code > 0 && length > 3 && line[3] == '-' ) + cont_resp = 0; + + if ( reply_code == 0 ) + { + AnalyzerViolation("non-numeric reply code", (const char*)data, length); + return; + } + else if ( reply_code < 100 ) + { + AnalyzerViolation("invalid reply code", (const char*)data, length); + return; + } + else if ( length > 3 && line[3] == '-' ) { // a continued reply pending_reply = reply_code; line = util::skip_whitespace(line + 4, end_of_line); cont_resp = 1; } + else if ( length > 3 && line[3] != ' ' ) + { + // This is a proper reply code, but there's no space after + // the reply code even though the line is long enough. + AnalyzerViolation("invalid reply line", (const char*)data, length); + return; + } else { // a self-contained reply - if ( reply_code > 0 ) - line += 3; - else - AnalyzerViolation("non-numeric reply code", (const char*)data, length); + line += 3; if ( line < end_of_line ) line = util::skip_whitespace(line, end_of_line); else line = end_of_line; - - cont_resp = 0; } }