From 69fa333e25c070689b707e261b31b08be82d9fc4 Mon Sep 17 00:00:00 2001
From: Leah Rowe <leah@libreboot.org>
Date: Thu, 1 Jun 2023 13:35:34 +0100
Subject: util/nvmutil: harden pledge/unveil calls (OpenBSD)

*Open* files at the start, then unveil. The same overall
behaviour is observed. In the case that invalid arguments
are given, simply opening a file does not cause much
performance impact (if any).

Restrict operations as early as possible in code.

Bonus:

writeGbeFile also hardened; if flags is O_RDONLY, it aborts.

Signed-off-by: Leah Rowe <leah@libreboot.org>
---
 util/nvmutil/nvmutil.c | 36 ++++++++++++++++++++++--------------
 util/nvmutil/nvmutil.h |  5 +++--
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/util/nvmutil/nvmutil.c b/util/nvmutil/nvmutil.c
index 38d34436..e8abe516 100644
--- a/util/nvmutil/nvmutil.c
+++ b/util/nvmutil/nvmutil.c
@@ -7,14 +7,18 @@ int
 main(int argc, char *argv[])
 {
 	xpledge("stdio rpath wpath unveil", NULL);
-	int flags = O_RDWR;
+	if (argc < 3)
+		err(errno = EINVAL, NULL);
+	if (strcmp(COMMAND, "dump") == 0)
+		flags = O_RDONLY;
+	openFiles(FILENAME);
+
 	void (*cmd)(void) = NULL;
 	const char *strMac = NULL, *strRMac = "??:??:??:??:??:??";
 
 	if (argc == 3) {
 		if (strcmp(COMMAND, "dump") == 0) {
-			xpledge("stdio rpath unveil", NULL);
-			flags = O_RDONLY;
+			xpledge("stdio", NULL);
 			cmd = &cmd_dump;
 		} else if (strcmp(COMMAND, "setmac") == 0) {
 			strMac = (char *) strRMac; /* random mac address */
@@ -42,16 +46,7 @@ main(int argc, char *argv[])
 
 	skipread[part ^ 1] = (cmd == &cmd_copy) | (cmd == &cmd_setchecksum)
 	    | (cmd == &cmd_brick);
-	readGbeFile(FILENAME, flags);
-
-	(void)rhex();
-	xunveil("/dev/urandom", "r");
-	if (flags == O_RDONLY) {
-		xpledge("stdio", NULL);
-	} else {
-		xpledge("stdio wpath unveil", NULL);
-		xunveil(FILENAME, "w");
-	}
+	readGbeFile(FILENAME);
 
 	if (strMac != NULL)
 		cmd_setmac(strMac); /* nvm gbe.bin setmac */
@@ -64,13 +59,24 @@ main(int argc, char *argv[])
 }
 
 void
-readGbeFile(const char *path, int flags)
+openFiles(const char *path)
 {
+	(void)rhex();
 	xopen(fd, path, flags);
 	if ((st.st_size != SIZE_8KB))
 		err(errno = ECANCELED, "File `%s` not 8KiB", path);
 	errno = errno != ENOTDIR ? errno : 0;
+	xunveil("/dev/urandom", "r");
+	if (flags != O_RDONLY) {
+		xunveil(path, "w");
+		xpledge("stdio wpath", NULL);
+	} else
+		xpledge("stdio", NULL);
+}
 
+void
+readGbeFile(const char *path)
+{
 	big_endian = ((uint8_t *) &test)[0] ^ 1;
 	gbe[1] = (gbe[0] = (size_t) buf) + SIZE_4KB;
 	for (int p = 0; p < 2; p++) {
@@ -252,6 +258,8 @@ xorswap_buf(int partnum)
 void
 writeGbeFile(const char *filename)
 {
+	if (flags == O_RDONLY)
+		err(ERR(), "Write aborted due to read-only mode: %s", filename);
 	if (gbeFileModified)
 		errno = 0;
 	for (int p = 0; p < 2; p++) {
diff --git a/util/nvmutil/nvmutil.h b/util/nvmutil/nvmutil.h
index 043e0f39..458b4338 100644
--- a/util/nvmutil/nvmutil.h
+++ b/util/nvmutil/nvmutil.h
@@ -13,7 +13,8 @@
 #include <string.h>
 #include <unistd.h>
 
-void readGbeFile(const char *path, int flags);
+void openFiles(const char *path);
+void readGbeFile(const char *path);
 void cmd_setmac(const char *strMac);
 int invalidMacAddress(const char *strMac, uint16_t *mac);
 uint8_t hextonum(char chs);
@@ -42,7 +43,7 @@ uint8_t *buf = (uint8_t *) &buf16;
 size_t nf = 128, gbe[2];
 uint8_t skipread[2] = {0, 0};
 
-int fd = -1, part, gbeFileModified = 0;
+int flags = O_RDWR, fd = -1, part, gbeFileModified = 0;
 uint8_t nvmPartModified[2] = {0, 0};
 
 int test = 1;
-- 
cgit v1.2.1