From b55d72e6ba3837ee8a49a95f8979049622aad7d7 Mon Sep 17 00:00:00 2001
From: Nalin Dahyabhai <nalin@redhat.com>
Date: Apr 10 2015 17:52:33 +0000
Subject: Fix potential segfault when parsing helper output


Fix a potential segfault while parsing the output of an enrollment
helper.  While cm_subproc_get_msg() ensures that the buffer we get back
is nul-terminated, and we depended on that, when we would memdup it to
keep the value around, we weren't continuing to make that guarantee, at
times leading to off-by-one reads.  From a report by Steve Neuharth.

---

diff --git a/src/submit-e.c b/src/submit-e.c
index 2b10e51..bdfd0ec 100644
--- a/src/submit-e.c
+++ b/src/submit-e.c
@@ -72,7 +72,12 @@ sanitize_cookie(void *parent, const char *value)
 	p = value + strcspn(value, "\r\n");
 	ret = talloc_strndup(parent, value, p - value);
 	if (ret != NULL) {
-		p += strspn(p, "\r\n");
+		if (*p == '\r') {
+			p++;
+		}
+		if (*p == '\n') {
+			p++;
+		}
 		if (*p != '\0') {
 			ret = talloc_strdup_append(ret, "\n");
 		}
@@ -149,6 +154,7 @@ cm_submit_e_ready(struct cm_submit_state *state)
 {
 	int status, ready, length;
 	const char *msg;
+	char *tmp;
 	struct cm_submit_external_state *estate;
 	struct cm_subproc_state *subproc;
 
@@ -190,8 +196,13 @@ cm_submit_e_ready(struct cm_submit_state *state)
 									       "\r\n"));
 					}
 					/* Save the output for processing later. */
-					estate->msg = talloc_memdup(estate, msg, length);
-					estate->msg_length = length;
+					tmp = talloc_size(estate, length + 1);
+					if (tmp != NULL) {
+						memcpy(tmp, msg, length);
+						tmp[length] = '\0';
+						estate->msg_length = length;
+					}
+					estate->msg = tmp;
 					/* Now launch the postprocessing step,
 					 * if we've got data to process. */
 					if (WEXITSTATUS(status) ==
@@ -227,8 +238,13 @@ cm_submit_e_ready(struct cm_submit_state *state)
 					if (WEXITSTATUS(status) == 0) {
 						/* Save the output for processing later. */
 						cm_log(1, "Child output:\n\"%.*s\"\n", length, msg);
-						estate->msg = talloc_memdup(estate, msg, length);
-						estate->msg_length = length;
+						tmp = talloc_size(estate, length + 1);
+						if (tmp != NULL) {
+							memcpy(tmp, msg, length);
+							tmp[length] = '\0';
+							estate->msg_length = length;
+						}
+						estate->msg = tmp;
 					} else{
 						cm_log(1, "Exit status was %d.\n",
 						       WEXITSTATUS(status));
diff --git a/tests/010-iterate/expected.out b/tests/010-iterate/expected.out
index b3b5e33..6bd4e02 100644
--- a/tests/010-iterate/expected.out
+++ b/tests/010-iterate/expected.out
@@ -410,6 +410,43 @@ CA_REJECTED
 CA_REJECTED
 -STOP-
 
+[Enroll until the CA rejects us after poll.]
+HAVE_KEY_PAIR
+-START-
+NEED_KEYINFO
+READING_KEYINFO
+HAVE_KEYINFO
+NEED_CSR
+-STOP-
+NEED_CSR
+-(RESET)-
+HAVE_KEYINFO
+-START-
+NEED_CSR
+GENERATING_CSR
+HAVE_CSR
+-STOP-
+HAVE_CSR
+-START-
+NEED_TO_SUBMIT
+SUBMITTING
+delay=1
+CA_WORKING
+NEED_TO_SUBMIT
+SUBMITTING
+NEED_TO_NOTIFY_REJECTION
+-STOP-
+NEED_TO_NOTIFY_REJECTION
+-START-
+NOTIFYING_REJECTION
+Request for certificate to be stored in file "$tmpdir/certfile3" rejected by CA.
+CA_REJECTED
+-STOP-
+CA_REJECTED
+-START-
+CA_REJECTED
+-STOP-
+
 [Enroll until the CA turns out to be unreachable.]
 HAVE_KEY_PAIR
 -START-
diff --git a/tests/010-iterate/run.sh b/tests/010-iterate/run.sh
index 83f6b3c..027395d 100755
--- a/tests/010-iterate/run.sh
+++ b/tests/010-iterate/run.sh
@@ -81,6 +81,21 @@ echo CA rejected us, must have been having a bad day.
 exit 2
 EOF
 chmod u+x ca-reject
+cat > ca-reject-second-time << EOF
+#!/bin/sh
+if test -z "\$CERTMONGER_CA_COOKIE" ; then
+	echo 1
+	echo Try again.
+	echo
+	echo Maybe later.
+	exit 5
+else
+	echo CA rejected us, must have been having a bad day.
+	echo cookie was "\$CERTMONGER_CA_COOKIE"
+	exit 2
+fi
+EOF
+chmod u+x ca-reject-second-time
 cat > ca-unreachable << EOF
 #!/bin/sh
 echo Could not contact CA.
@@ -491,6 +506,29 @@ $toolsdir/iterate ca5 entry5 NEED_TO_NOTIFY_REJECTION,NOTIFYING_REJECTION | sed 
 $toolsdir/iterate ca5 entry5 "" | sed 's@'"$tmpdir"'@$tmpdir@g'
 
 echo
+echo '[Enroll until the CA rejects us after poll.]'
+cat > entry5 << EOF
+id=Test
+ca_name=Meanie
+state=HAVE_KEY_PAIR
+key_storage_type=FILE
+key_storage_location=$tmpdir/keyfile
+cert_storage_type=FILE
+cert_storage_location=$tmpdir/certfile3
+notification_method=STDOUT
+EOF
+cat > ca5 << EOF
+id=Meanie
+ca_type=EXTERNAL
+ca_external_helper=$tmpdir/ca-reject-second-time
+EOF
+$toolsdir/iterate ca5 entry5 NEED_KEYINFO,READING_KEYINFO,HAVE_KEYINFO
+$toolsdir/iterate ca5 entry5 NEED_CSR,GENERATING_CSR
+$toolsdir/iterate ca5 entry5 NEED_TO_SUBMIT,SUBMITTING,CA_WORKING
+$toolsdir/iterate ca5 entry5 NEED_TO_NOTIFY_REJECTION,NOTIFYING_REJECTION | sed 's@'"$tmpdir"'@$tmpdir@g'
+$toolsdir/iterate ca5 entry5 "" | sed 's@'"$tmpdir"'@$tmpdir@g'
+
+echo
 echo '[Enroll until the CA turns out to be unreachable.]'
 cat > entry6 << EOF
 id=Test