Mirror/zeek
1
0
Fork 0
mirror of https://github.com/zeek/zeek.git synced 2025-10-02 06:38:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

ftp/main: Special case for intermediate reply lines

Browse source 
The medium.trace in the private external test suite contains one
session/server that violates the multi-line reply protocol and
happened to work out fairly well regardless due to how we looked
up the pending commands unconditionally before.

Continue to match up reply lines that "look like they contain status codes"
even if cont_resp = T. This still improves runtime for the OSS-Fuzz
generated test case and keeps the external baselines valid.

The affected session can be extracted as follows:

    zcat Traces/medium.trace.gz | tcpdump -r  - 'port 1491 and port 21'

We could push this into the analyzer, too, minimally the RFC says:

    > If an intermediary line begins with a 3-digit number, the Server
    > must pad the front  to avoid confusion.
This commit is contained in:
Arne Welzel 2023-02-20 19:32:29 +01:00
parent 1b3e8a611e
commit 7665e808a2
1 changed files with 39 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

40
scripts/base/protocols/ftp/main.zeek
View file

@ -320,10 +320,46 @@ event ftp_reply(c: connection, code: count, msg: string, cont_resp: bool) &prior
# Skip matching up intermediate reply lines (that do not have a
# valid status code) with pending commands. Because they may not
# have a proper status code, there's little point setting whatever
# their reply_code and reply_msg are on the command unless to ensure
# c$ftp$reply_code is actually populated with "something".
# their reply_code and reply_msg are on the command.
#
# There's a quirk: Some FTP servers return(ed?) replies like the
# following, violating the multi-line reply protocol:
#
# c: STOR intermol.ps
# s: 150 Opening ASCII mode data connection for 'intermol.ps'.
# s: 230- WARNING! 4 bare linefeeds received in ASCII mode
# s: File may not have transferred correctly.
# s: 226 Transfer complete.
#
# This is a multiline response started with 230-, but never finalized
# with the same status code. It should have been completed with
# "230 <some final message>", but instead was completed with "226 ...".
# This confuses our parser, returning cont_resp = T for all following
# server messages. This caused a regression as the current command wasn't
# updated for logging.
#
# The regex below is a best effort to keep existing behavior
# in face of such traffic. It matches on messages that look
# like valid status codes (starting with 3 digits followed by
# at least 10 ASCII characters).
#
# There's the following in RFC 959, so in the future we could push
# the detection/logic down into the parser instead of here.
#
# If an intermediary line begins with a 3-digit number, the Server
# must pad the front to avoid confusion.
#
if ( cont_resp && code == 0 && c$ftp?$reply_code )
{
if ( /^[1-9][0-9]{2} [[:print:]]{10}.*/ !in msg )
return;
else
{
# This might be worth a weird, but not sure it's
# worth it and how trigger happy it could be.
# Reporter::conn_weird("FTP_intermediate_line_with_reply_code", c, msg, "FTP");
}
}
c$ftp$cmdarg = get_pending_cmd(c$ftp$pending_commands, code, msg);
c$ftp$reply_code = code;