BUG/MINOR: cli: fix too many args detection for commands

d3f928944 ("BUG/MINOR: cli: Issue an error when too many args are passed
for a command") added a new check to prevent the command to run when
too many arguments are provided. In this case an error is reported.

However it turns out this check (despite marked for backports) was
ineffective prior to 20ec1de21 ("MAJOR: cli: Refacor parsing and
execution of pipelined commands") as 'p' pointer was reset to the end of
the buffer before the check was executed.

Now since 20ec1de21, the check works, but we have another issue: we may
read past initialized bytes in the buffer because 'p' pointer is always
incremented in a while loop without checking if we increment it past 'end'
(This was detected using valgrind)

To fix the issue introduced by 20ec1de21, let's only increment 'p' pointer
if p < end.

For 3.2 this is it, now for older versions, since d3f928944 was marked for
backport, a sligthly different approach is needed:

 - conditional p increment must be done in the loop (as in this patch)
 - max arg check must moved above "fill unused slots" comment where p is
   assigned to the end of the buffer

This patch should be backported with d3f928944.

(cherry picked from commit 976e0bd32f6d87fccf351edf1b5a18f3e0c5c6ab)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 71f72809886476e5cf3bf77631324e2d6b89071f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/cli.c b/src/cli.c
index a6fb34d..43e165b 100644 (file)
--- a/src/cli.c
+++ b/src/cli.c
@@ -784,7 +784,8 @@ static int cli_parse_request(struct appctx *appctx)
                                break;
                        }
                }
-               *p++ = 0;
+               if (p < end)
+                       *p++ = 0;
 
                /* unescape backslashes (\) */
                for (j = 0, k = 0; args[i][k]; k++) {