From 8596671dd55b283b8fd2dfba50ca131c675f2df4 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 5 May 2025 17:55:57 -0700 Subject: [PATCH 1/4] Fix invalid-read in FTP analyzer's parse_port method --- .typos.toml | 1 + src/analyzer/protocol/ftp/functions.bif | 23 ++++++++++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/.typos.toml b/.typos.toml index 4fcc006718..508f343d53 100644 --- a/.typos.toml +++ b/.typos.toml @@ -53,6 +53,7 @@ extend-ignore-identifiers-re = [ "complte_flag", # Existing use in exported record in base. "VidP(n|N)", # In SMB. "iin", # In DNP3. + "SCN[dioux]", # sccanf fixed-width identifiers "(ScValidatePnPService|ScSendPnPMessage)", # In DCE-RPC. "snet", # Used as shorthand for subnet in base scripts. "typ", diff --git a/src/analyzer/protocol/ftp/functions.bif b/src/analyzer/protocol/ftp/functions.bif index bac6e5b370..98ef814485 100644 --- a/src/analyzer/protocol/ftp/functions.bif +++ b/src/analyzer/protocol/ftp/functions.bif @@ -4,14 +4,15 @@ type ftp_port: record; %%{ #include "zeek/Reporter.h" -static zeek::RecordValPtr parse_port(const char* line) +static zeek::RecordValPtr parse_port(std::string_view line) { auto r = zeek::make_intrusive(zeek::BifType::Record::ftp_port); - int bytes[6]; - if ( line && sscanf(line, "%d,%d,%d,%d,%d,%d", - &bytes[0], &bytes[1], &bytes[2], - &bytes[3], &bytes[4], &bytes[5]) == 6 ) + int32_t bytes[6]; + if ( line.size() >= 11 && sscanf(line.data(), + "%" SCNd32 ",%" SCNd32 ",%" SCNd32 ",%" SCNd32 ",%" SCNd32 ",%" SCNd32, + &bytes[0], &bytes[1], &bytes[2], + &bytes[3], &bytes[4], &bytes[5]) == 6 ) { int good = 1; @@ -130,7 +131,7 @@ static zeek::RecordValPtr parse_eftp(const char* line) ## .. zeek:see:: parse_eftp_port parse_ftp_pasv parse_ftp_epsv fmt_ftp_port function parse_ftp_port%(s: string%): ftp_port %{ - return parse_port(s->CheckString()); + return parse_port(s->ToStdStringView()); %} ## Converts a string representation of the FTP EPRT command (see :rfc:`2428`) @@ -158,6 +159,10 @@ function parse_eftp_port%(s: string%): ftp_port function parse_ftp_pasv%(str: string%): ftp_port %{ const char* s = str->CheckString(); + + if ( str->Len() == 0 ) + return parse_port(""); + const char* line = strchr(s, '('); if ( line ) ++line; // move past '(' @@ -170,7 +175,11 @@ function parse_ftp_pasv%(str: string%): ftp_port ++line; // now points to first digit, or beginning of s } - return parse_port(line); + // Make sure we didn't step past the end of the string. + if ( ! line || ( line - s ) > str->Len() ) + return parse_port(""); + else + return parse_port(std::string_view{line}); %} ## Converts the result of the FTP EPSV command (see :rfc:`2428`) to an From c0b09665b9c7fa0544fe28d16d9a118086cacd08 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 5 May 2025 17:56:32 -0700 Subject: [PATCH 2/4] Fix undefined behavior in FTP analyzer's parse_port method --- src/analyzer/protocol/ftp/functions.bif | 34 ++++++++++--------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/analyzer/protocol/ftp/functions.bif b/src/analyzer/protocol/ftp/functions.bif index 98ef814485..d562375ecb 100644 --- a/src/analyzer/protocol/ftp/functions.bif +++ b/src/analyzer/protocol/ftp/functions.bif @@ -8,42 +8,36 @@ static zeek::RecordValPtr parse_port(std::string_view line) { auto r = zeek::make_intrusive(zeek::BifType::Record::ftp_port); + bool good = false; + uint32_t port = 0; + uint32_t addr = 0; + int32_t bytes[6]; if ( line.size() >= 11 && sscanf(line.data(), "%" SCNd32 ",%" SCNd32 ",%" SCNd32 ",%" SCNd32 ",%" SCNd32 ",%" SCNd32, &bytes[0], &bytes[1], &bytes[2], &bytes[3], &bytes[4], &bytes[5]) == 6 ) { - int good = 1; + good = true; for ( int i = 0; i < 6; ++i ) if ( bytes[i] < 0 || bytes[i] > 255 ) { - good = 0; + good = false; break; } - uint32_t addr = (bytes[0] << 24) | (bytes[1] << 16) | - (bytes[2] << 8) | bytes[3]; - uint32_t port = (bytes[4] << 8) | bytes[5]; - - // Since port is unsigned, no need to check for < 0. - if ( port > 65535 ) + if ( good ) { - port = 0; - good = 0; + addr = (bytes[0] << 24) | (bytes[1] << 16) | + (bytes[2] << 8) | bytes[3]; + port = (bytes[4] << 8) | bytes[5]; } + } - r->Assign(0, zeek::make_intrusive(htonl(addr))); - r->Assign(1, zeek::val_mgr->Port(port, TRANSPORT_TCP)); - r->Assign(2, good); - } - else - { - r->Assign(0, zeek::make_intrusive(uint32_t(0))); - r->Assign(1, zeek::val_mgr->Port(0, TRANSPORT_TCP)); - r->Assign(2, false); - } + r->Assign(0, zeek::make_intrusive(htonl(addr))); + r->Assign(1, zeek::val_mgr->Port(port, TRANSPORT_TCP)); + r->Assign(2, good); return r; } From 517dfff529991737a6042f7a79b44d4738f4d1e9 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 5 May 2025 18:21:00 -0700 Subject: [PATCH 3/4] Use bool instead of int flag in FTP analyzer's parse_eftp method --- src/analyzer/protocol/ftp/functions.bif | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/analyzer/protocol/ftp/functions.bif b/src/analyzer/protocol/ftp/functions.bif index d562375ecb..5af6346b93 100644 --- a/src/analyzer/protocol/ftp/functions.bif +++ b/src/analyzer/protocol/ftp/functions.bif @@ -49,7 +49,7 @@ static zeek::RecordValPtr parse_eftp(const char* line) int net_proto = 0; // currently not used zeek::IPAddr addr; // unspecified IPv6 address (all 128 bits zero) int port = 0; - int good = 0; + bool good = false; if ( line ) { @@ -61,12 +61,12 @@ static zeek::RecordValPtr parse_eftp(const char* line) if ( *line ) { - good = 1; + good = true; ++line; // skip delimiter net_proto = strtol(line, &next_delim, 10); if ( *next_delim != delimiter ) - good = 0; + good = false; line = next_delim; if ( *line ) @@ -78,7 +78,7 @@ static zeek::RecordValPtr parse_eftp(const char* line) if ( nptr == NULL ) { nptr = line + strlen(line); - good = 0; + good = false; } std::string s(line, nptr-line); // extract IP address @@ -95,12 +95,12 @@ static zeek::RecordValPtr parse_eftp(const char* line) ++line; // now the port port = strtol(line, &next_delim, 10); if ( *next_delim != delimiter ) - good = 0; + good = false; if ( port < 0 || port > 65535 ) { port = 0; - good = 0; + good = false; } } } From 9ae16a3db36e7ee7ac77d65842b3ec31b369971a Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Tue, 6 May 2025 08:46:38 -0700 Subject: [PATCH 4/4] Add extra input files to ftp fuzzer corpus --- src/fuzzers/corpora/ftp-corpus.zip | Bin 4069 -> 5391 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/src/fuzzers/corpora/ftp-corpus.zip b/src/fuzzers/corpora/ftp-corpus.zip index 21c3271b6147f189958665d25bc7aa71a564a99d..6bcffe1bf70ce714d6e2bf33e8c03483244f5fc5 100644 GIT binary patch delta 1439 zcmaDV->+34;LXe;!oa}5!7$5pS=97*_V2}jJaHfvV31)jwlp--&CDw(Er~bOE66Sh z4dG;9F42?90O8UKZU#n{uZ#=~U=xygKJX>-eBkE_G`M7V<)Y~oLk34Z22}H37geje z0L{J)Grt~Tfe|ql6!L>DxL{~_)$oELi&HGK-i8SaC+_T%3^>D~v5Z;Nkg>ZVMABJA zQhLTBZpoPu3%Czn4Y;gR!nvX|fSno5IbH4bSNKhVZgGaW2jLuJVw?l@?}dv%#{j+j zkn03{(b+RTJ-ugjcyxGrdP;o`261>Y%Ni@3mrzMM#R|5c3uqYwIJgV+>MIvY134g! z7TzX=>}L${4q*)N4+>E*)iKbq&@s}{H8C|aFw!yL0tGt4mO9I$lCnSxKp4#u(|TfT zQ3l%rG+4(_$5_YM$iU3R$ihg+5Nr=5@pJ$a&mCEZ)r*04OlgAKV~mk_0)PnzgrSM2 z9+rCEp10*XWFWzo@LJ*nUuk&8?vk>yjEwH%ANb~7yv)($=6bP7NpF!2zvQQ4h~fHx~J R1u`&j0wE93khg*$9sv4?V;ukh delta 106 zcmeCzdMZCzjoF#;+vY50J~kj#$$1Dw*$C`qocvc