portable-specific int overflow defence-in-depth

These too are unreachable, but we want the code to be safe regardless of
context. Reported by Yair Mizrahi @ JFrog

2 files changed, 16 insertions, 18 deletions

diff --git a/auth-pam.c b/auth-pam.c
index f5a06b1f..205715f0 100644
--- a/auth-pam.c
+++ b/auth-pam.c
@@ -848,7 +848,7 @@ sshpam_query(void *ctx, char **name, char **info,
 	size_t plen;
 	u_char type;
 	char *msg;
-	size_t len, mlen;
+	size_t len, mlen, nmsg = 0;
 	int r;
 
 	debug3("PAM: %s entering", __func__);
@@ -861,6 +861,8 @@ sshpam_query(void *ctx, char **name, char **info,
 	plen = 0;
 	*echo_on = xmalloc(sizeof(u_int));
 	while (ssh_msg_recv(ctxt->pam_psock, buffer) == 0) {
+		if (++nmesg > PAM_MAX_NUM_MSG)
+			fatal_f("too many query messages");
 		if ((r = sshbuf_get_u8(buffer, &type)) != 0 ||
 		    (r = sshbuf_get_cstring(buffer, &msg, &mlen)) != 0)
 			fatal("%s: buffer error: %s", __func__, ssh_err(r));
diff --git a/openbsd-compat/port-linux.c b/openbsd-compat/port-linux.c
index 4ca8c2b7..0457e28d 100644
--- a/openbsd-compat/port-linux.c
+++ b/openbsd-compat/port-linux.c
@@ -178,20 +178,20 @@ ssh_selinux_setup_pty(char *pwname, const char *tty)
 void
 ssh_selinux_change_context(const char *newname)
 {
-	int len, newlen;
-	char *oldctx, *newctx, *cx;
+	char *oldctx, *newctx, *cx, *cx2;
 	LogLevel log_level = SYSLOG_LEVEL_INFO;
 
 	if (!ssh_selinux_enabled())
 		return;
 
 	if (getcon(&oldctx) < 0) {
-		logit("%s: getcon failed with %s", __func__, strerror(errno));
+		logit_f("getcon failed with %s", strerror(errno));
 		return;
 	}
-	if ((cx = index(oldctx, ':')) == NULL || (cx = index(cx + 1, ':')) ==
-	    NULL) {
-		logit("%s: unparsable context %s", __func__, oldctx);
+	if ((cx = strchr(oldctx, ':')) == NULL ||
+	    (cx = strchr(cx + 1, ':')) == NULL ||
+	    (cx - oldctx) >= INT_MAX) {
+		logit_f("unparsable context %s", oldctx);
 		return;
 	}
 
@@ -203,18 +203,14 @@ ssh_selinux_change_context(const char *newname)
 	    sizeof(SSH_SELINUX_UNCONFINED_TYPE) - 1) == 0)
 		log_level = SYSLOG_LEVEL_DEBUG3;
 
-	newlen = strlen(oldctx) + strlen(newname) + 1;
-	newctx = xmalloc(newlen);
-	len = cx - oldctx + 1;
-	memcpy(newctx, oldctx, len);
-	strlcpy(newctx + len, newname, newlen - len);
-	if ((cx = index(cx + 1, ':')))
-		strlcat(newctx, cx, newlen);
-	debug3("%s: setting context from '%s' to '%s'", __func__,
-	    oldctx, newctx);
+	cx2 = strchr(cx + 1, ':');
+	xasprintf(&newctx, "%.*s%s%s", (int)(cx - oldctx + 1), oldctx,
+	    newname, cx2 == NULL ? "" : cx2);
+
+	debug3_f("setting context from '%s' to '%s'", oldctx, newctx);
 	if (setcon(newctx) < 0)
-		do_log2(log_level, "%s: setcon %s from %s failed with %s",
-		    __func__, newctx, oldctx, strerror(errno));
+		do_log2_f(log_level, "setcon %s from %s failed with %s",
+		    newctx, oldctx, strerror(errno));
 	free(oldctx);
 	free(newctx);
 }