[Asterisk-code-review] test.c: Avoid passing -1 to FD_* family of functions. (asterisk[20])

Friendly Automation asteriskteam at digium.com
Tue Feb 28 10:48:13 CST 2023


Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/19943 )

Change subject: test.c: Avoid passing -1 to FD_* family of functions.
......................................................................

test.c: Avoid passing -1 to FD_* family of functions.

This avoids buffer overflow errors when running tests that capture
output from child processes.

This also corrects a copypasta in an off-nominal error message.

Change-Id: Ib482847a3515364f14c7e7a0c0a4213851ddb10d
---
M main/test.c
1 file changed, 23 insertions(+), 4 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit




diff --git a/main/test.c b/main/test.c
index f06b557..9eefbd0 100644
--- a/main/test.c
+++ b/main/test.c
@@ -363,7 +363,7 @@
 	}
 
 	if (pipe(fd2) == -1) {
-		ast_log(LOG_ERROR, "Couldn't open stdout pipe: %s\n", strerror(errno));
+		ast_log(LOG_ERROR, "Couldn't open stderr pipe: %s\n", strerror(errno));
 		goto cleanup;
 	}
 
@@ -472,7 +472,10 @@
 			 */
 			n = select(nfds, &readfds, &writefds, NULL, NULL);
 
-			if (FD_ISSET(fd0[1], &writefds)) {
+			/* A version of FD_ISSET() that is tolerant of -1 file descriptors */
+#define SAFE_FD_ISSET(fd, setptr) ((fd) != -1 && FD_ISSET((fd), setptr))
+
+			if (SAFE_FD_ISSET(fd0[1], &writefds)) {
 				n = write(fd0[1], data, datalen);
 				if (n > 0) {
 					data += n;
@@ -485,7 +488,7 @@
 				}
 			}
 
-			if (FD_ISSET(fd1[0], &readfds)) {
+			if (SAFE_FD_ISSET(fd1[0], &readfds)) {
 				n = read(fd1[0], buf, sizeof(buf));
 				if (n > 0) {
 					fwrite(buf, sizeof(char), n, out);
@@ -494,7 +497,7 @@
 				}
 			}
 
-			if (FD_ISSET(fd2[0], &readfds)) {
+			if (SAFE_FD_ISSET(fd2[0], &readfds)) {
 				n = read(fd2[0], buf, sizeof(buf));
 				if (n > 0) {
 					fwrite(buf, sizeof(char), n, err);
@@ -502,6 +505,8 @@
 					zclose(fd2[0]);
 				}
 			}
+
+#undef SAFE_FD_ISSET
 		}
 		status = 1;
 

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19943
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 20
Gerrit-Change-Id: Ib482847a3515364f14c7e7a0c0a4213851ddb10d
Gerrit-Change-Number: 19943
Gerrit-PatchSet: 3
Gerrit-Owner: Sean Bright <sean at seanbright.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20230228/3a2ed2df/attachment-0001.html>


More information about the asterisk-code-review mailing list