From 2ef8ed9f0df53bddf14bdecc2ea48c2d233213cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= Date: Wed, 24 Jun 2026 11:34:05 +0200 Subject: =?UTF-8?q?substitutes:=20Ignore=20narinfos=20that=20don=E2=80=99t?= =?UTF-8?q?=20match=20the=20request=20[security=20fix].?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, by serving a valid, signed, and authorized narinfo that does not match what the client asked for, an attacker could cause users to get the wrong substitute. * guix/substitutes.scm (fetch-narinfos)[handle-narinfo-response]: Check whether the result of ‘read-narinfo’ matches ‘request’ and ignore it if it doesn’t. [do-fetch]: Likewise with ‘narinfo-from-file’. * tests/substitute.scm ("query narinfo that returns different store path") ("substitute, narinfo does not match requested store item"): New tests. Reported-by: Reepca Russelstein Signed-off-by: Ludovic Courtès Merges: #9665 --- guix/substitutes.scm | 36 ++++++++++++++++++++++-------------- tests/substitute.scm | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/guix/substitutes.scm b/guix/substitutes.scm index b2a79c2a48d..289a068dbf3 100644 --- a/guix/substitutes.scm +++ b/guix/substitutes.scm @@ -1,5 +1,5 @@ ;;; GNU Guix --- Functional package management for GNU -;;; Copyright © 2013-2021, 2023-2025 Ludovic Courtès +;;; Copyright © 2013-2021, 2023-2026 Ludovic Courtès ;;; Copyright © 2014 Nikita Karetnikov ;;; Copyright © 2018 Kyle Meyer ;;; Copyright © 2020 Christopher Baines @@ -230,7 +230,11 @@ it contains invalid store file names--and return the narinfo otherwise." (dump-port port (%make-void-port "w"))) (define (handle-narinfo-response request response port result) - (let* ((code (response-code response)) + (let* ((hash-part (basename + (string-drop-right (uri-path (request-uri request)) + (string-length ".narinfo")))) + (path (hash-part->path hash-part)) + (code (response-code response)) (len (response-content-length response)) (cache (response-cache-control response)) (ttl (and cache (assoc-ref cache 'max-age)))) @@ -240,21 +244,18 @@ it contains invalid store file names--and return the narinfo otherwise." ;; belong to the next response. (if (= code 200) ; hit (let ((narinfo (read-narinfo port url #:size len))) - (if narinfo + (if (and narinfo path + ;; Make sure we got what we asked for. + (string=? (narinfo-path narinfo) path)) (begin (cache-narinfo! url (narinfo-path narinfo) narinfo ttl) (cons narinfo result)) result)) - (let* ((path (uri-path (request-uri request))) - (hash-part (basename - (string-drop-right path 8)))) ;drop ".narinfo" - ;; Log the failing queries and indicate if it failed because the - ;; narinfo is being baked. - (let ((baking? - (assoc-ref (response-headers response) 'x-baking))) - (debug "could not fetch ~a~a ~a~a~%" - url path code - (if baking? " (baking)" ""))) + ;; Log the failing queries and indicate if it failed because the + ;; narinfo is being baked. + (let ((baking? (assoc-ref (response-headers response) 'x-baking))) + (debug "could not fetch ~a~a ~a~a~%" + url path code (if baking? " (baking)" "")) (if len (get-bytevector-n port len) (read-to-eof port)) @@ -288,7 +289,14 @@ it contains invalid store file names--and return the narinfo otherwise." (files (map (compose (cut string-append base <> ".narinfo") store-path-hash-part) paths))) - (filter-map (cut narinfo-from-file <> url) files))) + (filter-map (lambda (file path) + (let ((narinfo (narinfo-from-file file url))) + (and narinfo + ;; Make sure we got what we asked for. + (string=? (narinfo-path narinfo) path) + narinfo))) + files + paths))) (else (leave (G_ "~s: unsupported server URI scheme~%") (if uri (uri-scheme uri) url))))) diff --git a/tests/substitute.scm b/tests/substitute.scm index 8f32bfa7281..b749561347a 100644 --- a/tests/substitute.scm +++ b/tests/substitute.scm @@ -357,6 +357,29 @@ Deriver: foo.drv") (lambda () (guix-substitute "--query"))))))))) +(test-equal "query narinfo that returns different store path" + ;; The narinfo is valid and authorized but its 'StorePath' field points to a + ;; different store item. + "" + + (let ((prefix (string-append "StorePath: " (%store-prefix) + "/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-BAR +NarHash: sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +References: bar baz\n"))) + (with-narinfo (string-append prefix + "Signature: " (signature-field prefix) " +URL: example.nar +Compression: none +NarSize: 42 +Deriver: foo.drv") + (string-trim-both + (with-output-to-string + (lambda () + (with-input-from-string (string-append "have " (%store-prefix) + "/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-foo") + (lambda () + (guix-substitute "--query"))))))))) + (test-equal "query narinfo signed with authorized key" (string-append (%store-prefix) "/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-foo") @@ -893,6 +916,18 @@ System: mips64el-linux\n"))) (lambda () (false-if-exception (delete-file "substitute-retrieved"))))))) +(test-quit "substitute, narinfo does not match requested store item" + "no valid substitute" + (with-http-server `((200 ,(string-append %narinfo "Signature: " + (signature-field %narinfo)))) + (parameterize ((substitute-urls (list (%local-url)))) + ;; Narinfo is valid, signed, and authorized but requested item is + ;; /bbb…-bar and narinfo is for /aaa…-foo. + (request-substitution (string-append (%store-prefix) + "/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-bar") + "substitute-retrieved") + #f))) + (test-end "substitute") ;;; Local Variables: -- cgit v1.2.3