ftp: Do not base seq on number of pending commands

Previously, seq was computed as the result of |pending_commands|+1. This
opened the possibility to override queued commands, as well as logging
the same pending ftp reply multiple times.

For example, when commands 1, 2, 3 are pending, command 1 may be dequeued,
but the incoming command then receives seq 3 and overrides the already
pending command 3. The second scenario happens when ftp_reply() selected
command 3 as pending for logging, but is then followed by many ftp_request()
events. This resulted in command 3's response being logged for every
following ftp_request() over and over again.

Avoid both scenarios by tracking the command sequence as an absolute counter.
This commit is contained in:
Arne Welzel 2023-08-29 17:52:12 +02:00 committed by Tim Wojtulewicz
parent f6615753f1
commit b745556d36
5 changed files with 9 additions and 6 deletions

View file

@ -64,6 +64,9 @@ export {
## to are tracked here. ## to are tracked here.
pending_commands: PendingCmds; pending_commands: PendingCmds;
## Sequence number of previous command.
command_seq: count &default=0;
## Indicates if the session is in active or passive mode. ## Indicates if the session is in active or passive mode.
passive: bool &default=F; passive: bool &default=F;

View file

@ -165,7 +165,7 @@ function set_ftp_session(c: connection)
Conn::register_removal_hook(c, finalize_ftp); Conn::register_removal_hook(c, finalize_ftp);
# Add a shim command so the server can respond with some init response. # Add a shim command so the server can respond with some init response.
add_pending_cmd(c$ftp$pending_commands, "<init>", ""); add_pending_cmd(c$ftp$pending_commands, ++c$ftp$command_seq, "<init>", "");
} }
} }
@ -270,7 +270,7 @@ event ftp_request(c: connection, command: string, arg: string) &priority=5
# Queue up the new command and argument # Queue up the new command and argument
if ( |c$ftp$pending_commands| < max_pending_commands ) if ( |c$ftp$pending_commands| < max_pending_commands )
add_pending_cmd(c$ftp$pending_commands, command, arg); add_pending_cmd(c$ftp$pending_commands, ++c$ftp$command_seq, command, arg);
else else
Reporter::conn_weird("FTP_too_many_pending_commands", c, Reporter::conn_weird("FTP_too_many_pending_commands", c,
cat(|c$ftp$pending_commands|), "FTP"); cat(|c$ftp$pending_commands|), "FTP");

View file

@ -78,9 +78,9 @@ export {
}; };
} }
function add_pending_cmd(pc: PendingCmds, cmd: string, arg: string): CmdArg function add_pending_cmd(pc: PendingCmds, seq: count, cmd: string, arg: string): CmdArg
{ {
local ca = [$cmd = cmd, $arg = arg, $seq=|pc|+1, $ts=network_time()]; local ca = [$cmd = cmd, $arg = arg, $seq=seq, $ts=network_time()];
pc[ca$seq] = ca; pc[ca$seq] = ca;
return ca; return ca;

View file

@ -1 +1 @@
f0dfda82f616d478df1a290116b4b7efdd82f00a 9e2b8aaa316d6b818fa32c730fcc90ca8b401b68

View file

@ -1 +1 @@
eea2f9972f5b103048a40d07b3f14c41d92336bc 0e810e51b5fdeacfab53b5655023190c84f86f2e