commit ddf1151d18469ed361359892a83958f1f43c843d
from: Omar Polo <op@omarpolo.com>
date: Tue Oct 22 10:38:19 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 - 284a194b014a698450b40ee7eaef2e1d5334c607
commit + ddf1151d18469ed361359892a83958f1f43c843d
blob - 9f05cca64a73b00c1cebb028d16f3bd436fc1b1d
blob + e1898650c1c8e70d57548d096c1488b8d157641b
--- 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;
 }