From: Jason Ish Date: Fri, 22 Apr 2022 18:04:37 +0000 (-0600) Subject: ftp: truncate first segment if over max length X-Git-Tag: suricata-7.0.0-beta1~643 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9645285dff9eb8313db573d8603162a708736236;p=thirdparty%2Fsuricata.git ftp: truncate first segment if over max length The first segment was not limited to the configured maximum line length allowing it to be up to 65k. This could result in the next input length being negative, which while handled properly by the code, did trigger a debug validation assertion. The fix is to be consistent and apply the limit to the first segment as well, which does ensure the input_len could never be less than 0. Ticket #5281 --- diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c index b7ca50e771..85e428b0fb 100644 --- a/src/app-layer-ftp.c +++ b/src/app-layer-ftp.c @@ -387,6 +387,9 @@ static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state) } } + /* Should be guaranteed by the caller. */ + DEBUG_VALIDATE_BUG_ON(state->input_len <= 0); + uint8_t *lf_idx = memchr(state->input, 0x0a, state->input_len); if (lf_idx == NULL) { @@ -397,13 +400,18 @@ static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state) * if we see fragmentation then it's definitely something you * should alert about */ if (line_state->current_line_db == 0) { - line_state->db = FTPMalloc(state->input_len); + int32_t input_len = state->input_len; + if ((uint32_t)input_len > ftp_max_line_len) { + input_len = ftp_max_line_len; + state->current_line_truncated = true; + } + line_state->db = FTPMalloc(input_len); if (line_state->db == NULL) { return -1; } line_state->current_line_db = 1; - memcpy(line_state->db, state->input, state->input_len); - line_state->db_len = state->input_len; + memcpy(line_state->db, state->input, input_len); + line_state->db_len = input_len; } else if (!state->current_line_truncated) { int32_t input_len = state->input_len; if (line_state->db_len + input_len > ftp_max_line_len) {