BUG/MEDIUM: cli: Don't read a command or payload line into a full buffer

Commit e3c175871 ("BUG/MEDIUM: cli: Don't parse next command if input buffer
is empty") fixed a crash triggered when a command line exactly filling the
buffer was finished by a payload pattern, by no longer trying to parse a
next command when the input buffer was empty.

However this only addressed the case where no more data was available. When
a command line exactly fills the buffer and ends with a payload pattern, the
payload buffer is created with a size of 0 (no room left). If payload data
is then received on a subsequent read, the loop was not interrupted and
b_getline() was called with a length of 'b_room(buf) - 1'. Because b_room()
returns a size_t, an empty buffer made this length wrap around to SIZE_MAX,
and the line was copied past the end of the trash chunk, leading to a buffer
overflow.

To fix the issue, no attempt to read a command or a payload line is
performed when there is no room left in the destination buffer. In that
case, we fall back to the existing path that reallocates a bigger (dynamic)
payload buffer, which is guaranteed to be given a non-zero size, or reports
an error for the command line. This way b_getdelim() and b_getline() are
never called on a full buffer and the size_t underflow can no longer happen.

On older versions, 1-byte OOB write is also possible, but with a lower
severity.

This patch must be backported to 3.4, along with commit e3c175871. And it
should be adapted and backported to all other stable versions.

diff --git a/src/cli.c b/src/cli.c
index fafad7c83c3dd9a60d640284d098951f424dae25..26f83422419a2c35d73d96b6aef22c4b2bc49211 100644 (file)
--- a/src/cli.c
+++ b/src/cli.c
@@ -1062,10 +1062,13 @@ int cli_parse_cmdline(struct appctx *appctx)
                 */
                buf = (appctx->st1 & APPCTX_CLI_ST1_PAYLOAD) ? &appctx->cli_ctx.payload : appctx->cli_ctx.cmdline;
                str = b_tail(buf);
-               if (!(appctx->st1 & APPCTX_CLI_ST1_PAYLOAD))
-                       len = b_getdelim(&appctx->inbuf, 0, b_data(&appctx->inbuf), str, b_room(buf), "\n;", '\\');
-               else
-                       len = b_getline(&appctx->inbuf, 0, b_data(&appctx->inbuf), str, b_room(buf) - 1);
+               len = 0;
+               if (b_room(buf)) {
+                       if (!(appctx->st1 & APPCTX_CLI_ST1_PAYLOAD))
+                               len = b_getdelim(&appctx->inbuf, 0, b_data(&appctx->inbuf), str, b_room(buf), "\n;", '\\');
+                       else
+                               len = b_getline(&appctx->inbuf, 0, b_data(&appctx->inbuf), str, b_room(buf) - 1);
+               }
 
                if (!len) {
                        if (!b_room(buf) || (b_data(&appctx->inbuf) > b_room(buf) - 1)) {