From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:8:6d80::]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id mJ85DHWtemAPHwAAgWs5BA (envelope-from ) for ; Sat, 17 Apr 2021 11:42:13 +0200 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id ONH+BnWtemBmJgAA1q6Kng (envelope-from ) for ; Sat, 17 Apr 2021 09:42:13 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id A97E1124E7 for ; Sat, 17 Apr 2021 11:42:12 +0200 (CEST) Received: from localhost ([::1]:36120 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lXhSt-00025f-So for larch@yhetil.org; Sat, 17 Apr 2021 05:42:11 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:51004) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lXhSl-00025K-Hw for bug-guix@gnu.org; Sat, 17 Apr 2021 05:42:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:59000) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lXhSj-0004lD-SZ for bug-guix@gnu.org; Sat, 17 Apr 2021 05:42:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lXhSj-00009n-Qe for bug-guix@gnu.org; Sat, 17 Apr 2021 05:42:01 -0400 X-Loop: help-debbugs@gnu.org Subject: bug#47838: Grafter should check every byte it modifies, to avoid corruption Resent-From: Mark H Weaver Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Sat, 17 Apr 2021 09:42:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 47838 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: 47838@debbugs.gnu.org X-Debbugs-Original-To: bug-guix@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.1618652512585 (code B ref -1); Sat, 17 Apr 2021 09:42:01 +0000 Received: (at submit) by debbugs.gnu.org; 17 Apr 2021 09:41:52 +0000 Received: from localhost ([127.0.0.1]:42313 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lXhSW-00009I-2y for submit@debbugs.gnu.org; Sat, 17 Apr 2021 05:41:52 -0400 Received: from lists.gnu.org ([209.51.188.17]:37712) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lXhSL-000092-MN for submit@debbugs.gnu.org; Sat, 17 Apr 2021 05:41:47 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:50870) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lXhSJ-00022o-Um for bug-guix@gnu.org; Sat, 17 Apr 2021 05:41:37 -0400 Received: from world.peace.net ([64.112.178.59]:55326) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lXhSF-0004VA-IY for bug-guix@gnu.org; Sat, 17 Apr 2021 05:41:35 -0400 Received: from mhw by world.peace.net with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lXhSD-0001Sg-Ak; Sat, 17 Apr 2021 05:41:29 -0400 From: Mark H Weaver Date: Sat, 17 Apr 2021 05:39:42 -0400 Message-ID: <87eef98d1i.fsf@netris.org> MIME-Version: 1.0 Content-Type: text/plain Received-SPF: pass client-ip=64.112.178.59; envelope-from=mhw@netris.org; helo=world.peace.net X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-guix@gnu.org List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+larch=yhetil.org@gnu.org Sender: "bug-Guix" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1618652532; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:resent-cc:resent-from:resent-sender: resent-message-id:list-id:list-help:list-unsubscribe:list-subscribe: list-post; bh=Nr+Ocs/eymt/Hg7FzHeCKNCxQAF+jpKEEoHgJmhIk3o=; b=e+xZYL6OvRvmfSX596YDd7ySS1jwd3CV6vxNNwZ8BZnOW5s2eEeVUXRJKS7x6N3fgbEqAo yxEWhJbtaXt5xai1C3LN0nyA5U4sCfIqn+BHcUKUCwIDu8onsMsTNMNdgMGqs3mMMFNKJN nAZrO1Kz5UGP4Iu/JNvo4sPD+arydfjD0hTNTrsyr3rmZIAEf7n+bCwTNhJZGigpBOKGD2 bAU7hrbdWwB8Wikn/4dnyvlFl2QinK/EWby9wHVJUIsVztZR5Q/B5822q+r8jJEhAVhvmO rxk15+lLxCbOQZHh0cHPITbrZX1RyY419nNkDAWu3sKLxszn9d/Vv/+un4Zx4w== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1618652532; a=rsa-sha256; cv=none; b=iK1dZ/0kCzNIzSL+h6ZdiFMUe/wKFrQOrMOrPTdo5dPasQZzkoW1KLahy48H3odDnL4kvv Bjnn9qDAV6vGVWhSMdUW1UCPpCTlRF8uxtOjVRuH1THSWxFAnaoyc/O04QpFsLUcAMbWA0 6BUhVTRZBDhaQb/W89p+meDB6zEWGdifOhx0EXqqZzKb+WyAzz5BfHORb3qlH/Ip5OsZmx MsippB9FyqGA04QSKsZbrqOxc8BAxf4zVf1cJgTr8f8I5gBM5FO2hz/lz9ZKPwfFP563M9 xG0MGR3EUOee5cgymhnyyv5tuncMuMNDa8CWJqNqanvfLwVjrRON8PlzlLAuhg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; spf=pass (aspmx1.migadu.com: domain of bug-guix-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=bug-guix-bounces@gnu.org X-Migadu-Spam-Score: -0.94 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of bug-guix-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=bug-guix-bounces@gnu.org X-Migadu-Queue-Id: A97E1124E7 X-Spam-Score: -0.94 X-Migadu-Scanner: scn0.migadu.com X-TUID: pwLCYuzZwwtf Recall that the grafting code performs a set of substitutions, replacing store item names (i.e. file names in /gnu/store) with replacement store items of the same length, with rules like: "fx3979c88s9yxdbchyf36qryawgzpwb5-libx11-1.6.10" => "rwkqxykm91a75w9afhb41saj0dmf30hw-libx11-1.6.12". The grafting code currently only checks the first 33 bytes, consisting of the nix-base32 hash and the "-". It *assumes* that the remainder of the associated store item name immediately follows, and blindly writes the replacement string over whatever is there. Here's a real-world example of silent corruption caused by this in a Racket .zo file, before commit 834aa48504a24f0c79e858fc295edbf63815a408 which patched Racket to avoid embedding this store reference: --8<---------------cut here---------------start------------->8--- mhw@jojen ~$ diff -u <(hexdump -C $(guix build racket --no-grafts)/share/racket/pkgs/gui-lib/mred/private/wx/gtk/compiled/utils_rkt.zo) \ <(hexdump -C $(guix build racket )/share/racket/pkgs/gui-lib/mred/private/wx/gtk/compiled/utils_rkt.zo) --- /dev/fd/63 2021-04-15 04:36:01.240427788 -0400 +++ /dev/fd/62 2021-04-15 04:36:01.240427788 -0400 @@ -2047,11 +2047,11 @@ 00007fe0 49 8b 6f 0b 08 00 09 d0 02 2f d7 fe d0 02 07 f2 |I.o....../......| 00007ff0 02 0b 00 62 12 04 00 12 12 05 00 3e 12 06 00 17 |...b.......>....| 00008000 12 07 3d 02 f0 28 32 02 04 75 6e 69 78 00 1e 26 |..=..(2..unix..&| -00008010 5a 2f 67 6e 75 2f 73 74 6f 72 65 2f 69 72 6a 61 |Z/gnu/store/irja| -00008020 6e 35 77 71 37 6a 32 35 66 61 32 6d 36 6e 32 78 |n5wq7j25fa2m6n2x| -00008030 68 6c 38 6d 67 6c 73 61 71 78 6e 34 2d 49 02 02 |hl8mglsaqxn4-I..| -00008040 a5 02 fd 01 2b 73 76 67 2d 32 2e 34 30 2e 30 2f |....+svg-2.40.0/| -00008050 6c 69 62 2f c2 02 39 2e 73 6f 9b 02 1d 43 9b 02 |lib/..9.so...C..| +00008010 5a 2f 67 6e 75 2f 73 74 6f 72 65 2f 36 66 32 30 |Z/gnu/store/6f20| +00008020 38 64 61 6b 32 77 64 62 30 61 72 31 68 6e 38 79 |8dak2wdb0ar1hn8y| +00008030 6b 31 39 30 79 77 67 67 69 77 33 63 2d 67 64 6b |k190ywggiw3c-gdk| +00008040 2d 70 69 78 62 75 66 2b 73 76 67 2d 32 2e 34 30 |-pixbuf+svg-2.40| +00008050 2e 30 62 2f c2 02 39 2e 73 6f 9b 02 1d 43 9b 02 |.0b/..9.so...C..| 00008060 57 30 07 01 02 0e 35 00 05 a2 02 c3 12 08 0c 26 |W0....5........&| 00008070 00 18 12 09 02 2c 84 00 55 02 0f 54 02 09 13 7f |.....,..U..T....| 00008080 54 02 1b 49 54 02 15 32 54 02 15 1c 54 02 1f 01 |T..IT..2T...T...| --8<---------------cut here---------------end--------------->8--- For the record, when I originally wrote this fast(er) grafting code (commit 5a1add373ab427a3b336981d857252e703a9f8d1), by design it only rewrote the hashes, and so naturally it had the following desirable property: it never overwrote any byte without first checking it against an expected value. Later, starting in commit 57bdd79e485801ccf405ca7389bd099809fe5d67, the grafting code was modified to allow rewriting the entire store item name (notably including the version number). Unfortunately, although the set of overwritten bytes was extended past the "-", the set of bytes *checked* was left unchanged, and thus the aforementioned desirable property was lost. I think we ought to restore that property, to avoid silent corruptions such as the example above. Ideally, an error should be raised if the 'hash+dash' pattern is present but not followed by the expected bytes, so that we will be alerted to a problem. It would be even better to detect hashes by themselves, even if not followed by a dash, or even non-trivial substrings of hashes, in order to help alert us to problems like this. I'll try to find the time to work on this soon, but no promises. Mark