commit 6ddb5901d160201fffde89a37085637a9789452b
from: Omar Polo <op@omarpolo.com>
via: Thomas Adam <thomas@xteddy.org>
date: Tue Oct 22 16:27:52 2024 UTC

gotwebd: fix out of bound access while handling the config

For the first 13 iterations of the loop we read behind the start of
the array.  Instead of just adding a range check, rewrite the code to
split the loop in two so it's easier to follow.

Reported by Timo Myyrä
ok stsp@

commit - e1d6c2b511cd22a5ef00c93a3deee9274643a1c6
commit + 6ddb5901d160201fffde89a37085637a9789452b
blob - c1e15a921e4a3bba07a8dcf1d04b0084fbe0b700
blob + dabac41a85336b3eb98bd8e26bf2a97898360bc8
--- gotwebd/config.c
+++ gotwebd/config.c
@@ -191,33 +191,28 @@ config_setfd(struct gotwebd *env)
 int
 config_getfd(struct gotwebd *env, struct imsg *imsg)
 {
-	int match = 0, i, j;
-	const int nfds = GOTWEB_PACK_NUM_TEMPFILES + PRIV_FDS__MAX;
+	int i;
 
 	if (imsg_get_len(imsg) != 0)
 		fatalx("%s: wrong size", __func__);
 
-	for (i = 0; i < nfds; i++) {
-		if (i < PRIV_FDS__MAX && env->priv_fd[i] == -1) {
+	for (i = 0; i < nitems(env->priv_fd); ++i) {
+		if (env->priv_fd[i] == -1) {
 			env->priv_fd[i] = imsg_get_fd(imsg);
 			log_debug("%s: assigning priv_fd %d",
 			    __func__, env->priv_fd[i]);
-			match = 1;
-			break;
+			return 0;
 		}
+	}
 
-		j = i - PRIV_FDS__MAX;
-		if (env->pack_fds[j] == -1) {
-			env->pack_fds[j] = imsg_get_fd(imsg);
+	for (i = 0; i < nitems(env->pack_fds); ++i) {
+		if (env->pack_fds[i] == -1) {
+			env->pack_fds[i] = imsg_get_fd(imsg);
 			log_debug("%s: assigning pack_fd %d",
-			    __func__, env->pack_fds[j]);
-			match = 1;
-			break;
+			    __func__, env->pack_fds[i]);
+			return 0;
 		}
 	}
 
-	if (match)
-		return 0;
-	else
-		return 1;
+	return 1;
 }