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?= <aurelien.nepht...@corp.ovh.com>
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 <aurelien.nepht...@corp.ovh.com>
---
 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

Reply via email to