Hello, Here is a small patch to fix a potential crash when using CLI_ST_PRINT_FREE in an error path in the 'map' code. The problematic part is in the 'add' feature but all other usages have ben modified the same way to be consistent.
It also adds a missing error message in the SSL code when the loading of an OCSP response failed. The error path always sets the 'err' variable but it's a good idea to handle the case where it does not in the event that future code forgot to fill it. -- Aurélien.
>From e91962c02cdc0ae48ff59066cdc0f22e1063b496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Nephtali?= <[email protected]> Date: Sun, 15 Apr 2018 07:39:54 +0200 Subject: [PATCH] BUG/MINOR: cli: Ensure appctx->ctx.cli.err is always set when using CLI_ST_PRINT_FREE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the CLI, appctx->ctx.cli.err must point to a valid string when using the state CLI_ST_PRINT_FREE otherwise a crash will happen. In the map code, one error path does not fill the 'err' variable (pat_ref_add() -> pat_ref_push()) and in the SSL code it is correct but it lacks a default error message if the function called does not fill 'err'. In both cases only use CLI_ST_PRINT_FREE if 'err' is not NULL otherwise use a static default message. It should be backported to 1.8. Signed-off-by: Aurélien Nephtali <[email protected]> --- src/map.c | 38 ++++++++++++++++++++++++++++---------- src/ssl_sock.c | 5 +++++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/map.c b/src/map.c index 9313dc87e..6cf6e4a87 100644 --- a/src/map.c +++ b/src/map.c @@ -723,15 +723,21 @@ static int cli_parse_set_map(char **args, struct appctx *appctx, void *private) return 1; } - /* Try to delete the entry. */ + /* Try to modify the entry. */ err = NULL; HA_SPIN_LOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock); if (!pat_ref_set_by_id(appctx->ctx.map.ref, ref, args[4], &err)) { HA_SPIN_UNLOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock); - if (err) + if (err) { memprintf(&err, "%s.\n", err); - appctx->ctx.cli.err = err; - appctx->st0 = CLI_ST_PRINT_FREE; + appctx->ctx.cli.err = err; + appctx->st0 = CLI_ST_PRINT_FREE; + } + else { + appctx->ctx.cli.severity = LOG_ERR; + appctx->ctx.cli.msg = "Failed to update an entry.\n"; + appctx->st0 = CLI_ST_PRINT; + } return 1; } HA_SPIN_UNLOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock); @@ -744,10 +750,16 @@ static int cli_parse_set_map(char **args, struct appctx *appctx, void *private) HA_SPIN_LOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock); if (!pat_ref_set(appctx->ctx.map.ref, args[3], args[4], &err)) { HA_SPIN_UNLOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock); - if (err) + if (err) { memprintf(&err, "%s.\n", err); - appctx->ctx.cli.err = err; - appctx->st0 = CLI_ST_PRINT_FREE; + appctx->ctx.cli.err = err; + appctx->st0 = CLI_ST_PRINT_FREE; + } + else { + appctx->ctx.cli.severity = LOG_ERR; + appctx->ctx.cli.msg = "Failed to update an entry.\n"; + appctx->st0 = CLI_ST_PRINT; + } return 1; } HA_SPIN_UNLOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock); @@ -829,10 +841,16 @@ static int cli_parse_add_map(char **args, struct appctx *appctx, void *private) ret = pat_ref_add(appctx->ctx.map.ref, args[3], NULL, &err); HA_SPIN_UNLOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock); if (!ret) { - if (err) + if (err) { memprintf(&err, "%s.\n", err); - appctx->ctx.cli.err = err; - appctx->st0 = CLI_ST_PRINT_FREE; + appctx->ctx.cli.err = err; + appctx->st0 = CLI_ST_PRINT_FREE; + } + else { + appctx->ctx.cli.severity = LOG_ERR; + appctx->ctx.cli.msg = "Failed to add an entry.\n"; + appctx->st0 = CLI_ST_PRINT; + } return 1; } diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 8151cb381..23ad35b18 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -8588,6 +8588,11 @@ static int cli_parse_set_ocspresponse(char **args, struct appctx *appctx, void * appctx->ctx.cli.err = err; appctx->st0 = CLI_ST_PRINT_FREE; } + else { + appctx->ctx.cli.severity = LOG_ERR; + appctx->ctx.cli.msg = "Failed to update OCSP response.\n"; + appctx->st0 = CLI_ST_PRINT; + } return 1; } appctx->ctx.cli.severity = LOG_INFO; -- 2.11.0
