Prevent patch(1) from scribbling all over the place.
authorflorian <florian@openbsd.org>
Sat, 15 Jul 2023 10:42:54 +0000 (10:42 +0000)
committerflorian <florian@openbsd.org>
Sat, 15 Jul 2023 10:42:54 +0000 (10:42 +0000)
Arguably the only sensible use of patch(1) is changing files in the
current working directory and subdirectories.

However, patch(1) has this anti-feature, or dare I say bug, where it
will happily follow "../" upwards and outside of the current working
directory to find files to change. All it takes is a line like
+++ ../../../../home/florian/.ssh/authorized_keys
in the patchfile.

patch(1) operates on untrusted input and it already pledge(2)'ed to
not execute arbitrary programs, but of course it needs to write
files.

A simple unveil(".", "rwc") restricts patch(1) to its current working
directory.

We also need to allow /tmp and potentially the output file and reject
file if given on the command line. But those paths are safe.

input op, deraadt
OK millert, sthen

usr.bin/patch/patch.c

index a9740f0..67da2bd 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: patch.c,v 1.72 2023/07/12 11:26:13 tb Exp $   */
+/*     $OpenBSD: patch.c,v 1.73 2023/07/15 10:42:54 florian Exp $      */
 
 /*
  * patch - a program to apply diffs to original files
@@ -149,7 +149,7 @@ main(int argc, char *argv[])
        const   char *tmpdir;
        char    *v;
 
-       if (pledge("stdio rpath wpath cpath tmppath fattr", NULL) == -1) {
+       if (pledge("stdio rpath wpath cpath tmppath fattr unveil", NULL) == -1) {
                perror("pledge");
                my_exit(2);
        }
@@ -204,6 +204,38 @@ main(int argc, char *argv[])
        Argc = argc;
        Argv = argv;
        get_some_switches();
+       if (unveil(tmpdir, "rwc") == -1) {
+               perror("unveil");
+               my_exit(2);
+       }
+       if (outname != NULL)
+               if (unveil(outname, "rwc") == -1) {
+                       perror("unveil");
+                       my_exit(2);
+               }
+       if (filearg[0] != NULL)
+               if (unveil(filearg[0], "rwc") == -1) {
+                       perror("unveil");
+                       my_exit(2);
+               }
+       if (filearg[1] != NULL)
+               if (unveil(filearg[1], "r") == -1) {
+                       perror("unveil");
+                       my_exit(2);
+               }
+       if (unveil(".", "rwc") == -1) {
+               perror("unveil");
+               my_exit(2);
+       }
+       if (*rejname != '\0')
+               if (unveil(rejname, "rwc") == -1) {
+                       perror("unveil");
+                       my_exit(2);
+               }
+       if (pledge("stdio rpath wpath cpath tmppath fattr", NULL) == -1) {
+               perror("pledge");
+               my_exit(2);
+       }
 
        if (backup_type == none) {
                if ((v = getenv("PATCH_VERSION_CONTROL")) == NULL)