From: William Lallemand <wlallemand@haproxy.com>
Date: Tue, 14 Jun 2016 15:45:18 +0000 (+0200)
Subject: BUG/MEDIUM: dumpstats: undefined behavior in stats_tlskeys_list()
X-Git-Tag: v1.7-dev4~75
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cf9e78879008ee3ec1777fa2d0b45661acbb9380;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: dumpstats: undefined behavior in stats_tlskeys_list()

The function stats_tlskeys_list() can meet an undefined behavior when
called with appctx->st2 == STAT_ST_LIST, indeed the ref pointer is used
uninitialized.

However this function was using NULL in appctx->ctx.tlskeys.ref as a
flag to dump every tickets from every references.  A real flag
appctx->ctx.tlskeys.dump_all is now used for this behavior.

This patch delete the 'ref' variable and use appctx->ctx.tlskeys.ref
directly.
---

diff --git a/include/types/applet.h b/include/types/applet.h
index eb15456063..0543f94998 100644
--- a/include/types/applet.h
+++ b/include/types/applet.h
@@ -106,6 +106,7 @@ struct appctx {
 		} map;
 #if (defined SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB && TLS_TICKETS_NO > 0)
 		struct {
+			int dump_all;
 			struct tls_keys_ref *ref;
 		} tlskeys;
 #endif
diff --git a/src/dumpstats.c b/src/dumpstats.c
index 05d30aa631..b9f85dafaa 100644
--- a/src/dumpstats.c
+++ b/src/dumpstats.c
@@ -1446,8 +1446,10 @@ static int stats_sock_parse_request(struct stream_interface *si, char *line)
 		}
 		else if (strcmp(args[1], "tls-keys") == 0) {
 #if (defined SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB && TLS_TICKETS_NO > 0)
+			appctx->ctx.tlskeys.dump_all = 0;
 			/* no parameter, shows only file list */
 			if (!*args[2]) {
+				appctx->ctx.tlskeys.dump_all = 1;
 				appctx->st2 = STAT_ST_INIT;
 				appctx->st0 = STAT_CLI_O_TLSK;
 				return 1;
@@ -1456,6 +1458,7 @@ static int stats_sock_parse_request(struct stream_interface *si, char *line)
 			if (args[2][0] == '*') {
 				/* list every TLS ticket keys */
 				appctx->ctx.tlskeys.ref = NULL;
+				appctx->ctx.tlskeys.dump_all = 1;
 			} else {
 				appctx->ctx.tlskeys.ref = tlskeys_ref_lookup_ref(args[2]);
 				if(!appctx->ctx.tlskeys.ref) {
@@ -6131,7 +6134,6 @@ static int stats_dump_full_sess_to_buffer(struct stream_interface *si, struct st
 #if (defined SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB && TLS_TICKETS_NO > 0)
 static int stats_tlskeys_list(struct stream_interface *si) {
 	struct appctx *appctx = __objt_appctx(si->end);
-	struct tls_keys_ref *ref;
 
 	switch (appctx->st2) {
 	case STAT_ST_INIT:
@@ -6151,44 +6153,42 @@ static int stats_tlskeys_list(struct stream_interface *si) {
 			return 0;
 		}
 
-		ref = appctx->ctx.tlskeys.ref;
 
 		/* Now, we start the browsing of the references lists.
 		 * Note that the following call to LIST_ELEM return bad pointer. The only
 		 * available field of this pointer is <list>. It is used with the function
 		 * tlskeys_list_get_next() for retruning the first available entry
 		 */
-		if (ref == NULL) {
-			ref = LIST_ELEM(&tlskeys_reference, struct tls_keys_ref *, list);
-			ref = tlskeys_list_get_next(ref, &tlskeys_reference);
+		if (appctx->ctx.tlskeys.ref == NULL) {
+			appctx->ctx.tlskeys.ref = LIST_ELEM(&tlskeys_reference, struct tls_keys_ref *, list);
+			appctx->ctx.tlskeys.ref = tlskeys_list_get_next(appctx->ctx.tlskeys.ref, &tlskeys_reference);
 		}
 
 		appctx->st2 = STAT_ST_LIST;
 		/* fall through */
 
 	case STAT_ST_LIST:
-		while (ref) {
+		while (appctx->ctx.tlskeys.ref) {
 			int i;
-			int head = ref->tls_ticket_enc_index;
+			int head = appctx->ctx.tlskeys.ref->tls_ticket_enc_index;
 
 			chunk_reset(&trash);
 			if (appctx->st0 == STAT_CLI_O_TLSK_ENT)
 				chunk_appendf(&trash, "# ");
-			chunk_appendf(&trash, "%d (%s)\n", ref->unique_id,
-			              ref->filename);
-
+			chunk_appendf(&trash, "%d (%s)\n", appctx->ctx.tlskeys.ref->unique_id,
+			              appctx->ctx.tlskeys.ref->filename);
 			if (appctx->st0 == STAT_CLI_O_TLSK_ENT) {
 				for (i = 0; i < TLS_TICKETS_NO; i++) {
 					struct chunk *t2 = get_trash_chunk();
 					int b64_len;
 
 					chunk_reset(t2);
-					b64_len = a2base64((char *)(ref->tlskeys + (head + 2 + i) % TLS_TICKETS_NO),
+					b64_len = a2base64((char *)(appctx->ctx.tlskeys.ref->tlskeys + (head + 2 + i) % TLS_TICKETS_NO),
 					                   sizeof(struct tls_sess_key), t2->str, t2->size);
 					if (b64_len < 0)
 						return 0;
 					t2->len = b64_len;
-					chunk_appendf(&trash, "%d.%d %s\n", ref->unique_id, i, t2->str);
+					chunk_appendf(&trash, "%d.%d %s\n", appctx->ctx.tlskeys.ref->unique_id, i, t2->str);
 				}
 			}
 			if (bi_putchk(si_ic(si), &trash) == -1) {
@@ -6199,11 +6199,11 @@ static int stats_tlskeys_list(struct stream_interface *si) {
 				return 0;
 			}
 
-			if (appctx->ctx.tlskeys.ref) /* don't display everything if don't null */
+			if (appctx->ctx.tlskeys.dump_all == 0) /* don't display everything if not necessary */
 				break;
 
 			/* get next list entry and check the end of the list */
-			ref = tlskeys_list_get_next(ref, &tlskeys_reference);
+			appctx->ctx.tlskeys.ref = tlskeys_list_get_next(appctx->ctx.tlskeys.ref, &tlskeys_reference);
 
 		}