From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id 917F99C16; Wed, 10 Sep 2025 18:55:03 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 16DA79AFE; Wed, 10 Sep 2025 18:55:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DMARC_MISSING,RCVD_IN_DNSWL_LOW,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=4.0.1 Received: from fout-a3-smtp.messagingengine.com (fout-a3-smtp.messagingengine.com [103.168.172.146]) by atuin.qyliss.net (Postfix) with ESMTPS id B521F9BE4 for ; Wed, 10 Sep 2025 18:54:59 +0000 (UTC) Received: from phl-compute-08.internal (phl-compute-08.internal [10.202.2.48]) by mailfout.phl.internal (Postfix) with ESMTP id AD02DEC03C5; Wed, 10 Sep 2025 14:54:58 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-08.internal (MEProxy); Wed, 10 Sep 2025 14:54:58 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h=cc :cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1757530498; x=1757616898; bh=4oHvo+J4OO wZP5xeSuKLAdtzbQZW+JD4rBjkQ374tHc=; b=NB6dQQ503P6f+pnQjo3b6QBshj Nk3gJQ63LfQUx5T8vp2k6ckPEWQnHww1u/NftxaRzvRN8ngKIyNhy+0CfiVhJua2 I1RKxBTSy3bcipz3LCTla1KZCZSw4+YipDmZx8R3U2tEUe2qJUrGqwMfZGnh9yWI CeJtVgFoR4z8fAwogrdwfa7PtxJh+7QnXVXVRybHcYn1Ac7u41Ugmfzh5oyo4mYs qW5ruj5XPT16yMQrlA8xeXaF1j3QCXD6mTxumQZ35MV9U2CckrFL88DCqX7327oo hK30iJljA+kwAbWnot41Z/nRXbsad/Q31jpPEck/zIieSFGfOacLFZepLqLw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1757530498; x=1757616898; bh=4oHvo+J4OOwZP5xeSuKLAdtzbQZW+JD4rBj kQ374tHc=; b=UjTyVzpSV702EqDD8Yegs58ba9+GRorQzW8boM9HtXLVZkmCtET 5z2w2I77huAluXYqioLPDXNzp7VcNP1bjG7Fx+8sLycyb46IV0cmz2xf/K7cBwrJ 9EytspoGaB8gVpnyauajAz0f6YVvMN5kTvzwyRla+XD+XfCMDDY+mR4c5L0Zyy6X KR+WLkFSok0bIdfkD5plFB/6RdCYr5mEklli6VMox/b+yXjjn8SU/LivKOPhY/69 zhl5Je2cDOdmM08lcxbyxhOenbCz3TTQeL4USB93kRBTrO7HzmWJaOPROOZzDsmk 3BsNpttw4+I6wqo2Zyruxqh34ypWb1JPbHg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggddvgedtiecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpuffrtefokffrpgfnqfghnecuuegr ihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjug hrpefhvfevufgjfhffkfggtgesghdtreertddtjeenucfhrhhomheptehlhihsshgrucft ohhsshcuoehhihesrghlhihsshgrrdhisheqnecuggftrfgrthhtvghrnhepteehvedugf ejgfehhfeijeduleekleejgedvkeeuuefhhfegvdevfeetveegteeinecuvehluhhsthgv rhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhephhhisegrlhihshhsrgdrih hspdhnsggprhgtphhtthhopedvpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopegu vghmihhosggvnhhouhhrsehgmhgrihhlrdgtohhmpdhrtghpthhtohepuggvvhgvlhessh hpvggtthhruhhmqdhoshdrohhrgh X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 10 Sep 2025 14:54:58 -0400 (EDT) Received: by mbp.qyliss.net (Postfix, from userid 1000) id 2E00116F2101; Wed, 10 Sep 2025 20:54:47 +0200 (CEST) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH 04/20] scripts/make-erofs.sh: Validate all paths In-Reply-To: <80ecbfb5-441e-4fc6-96ca-c765701ea523@gmail.com> References: <20250904-systemd-v1-0-2a63b790a913@gmail.com> <20250904-systemd-v1-4-2a63b790a913@gmail.com> <871pohl4rr.fsf@alyssa.is> <80ecbfb5-441e-4fc6-96ca-c765701ea523@gmail.com> Date: Wed, 10 Sep 2025 20:54:46 +0200 Message-ID: <87v7lq87ex.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: FFIWVPYKAWBYY3ZRUZAE6AVVH3GXMGWP X-Message-ID-Hash: FFIWVPYKAWBYY3ZRUZAE6AVVH3GXMGWP X-MailFrom: hi@alyssa.is X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-devel.spectrum-os.org-0; header-match-devel.spectrum-os.org-1; header-match-devel.spectrum-os.org-2; header-match-devel.spectrum-os.org-3; header-match-devel.spectrum-os.org-4; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Spectrum OS Development X-Mailman-Version: 3.3.9 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Demi Marie Obenour writes: > On 9/8/25 04:36, Alyssa Ross wrote: >> Demi Marie Obenour writes: >>=20 >>> This isn't a security feature as the input is trusted, but it might >>> catch some bugs in the future. Additionally, it will allow replacing an >>> external command with builtin string manipulation, as paths that the >>> builtin manipulation would mishandle will instead be rejected. >>=20 >> In general this feels a bit overkill to me, but it depends =E2=80=94 hav= e you >> encountered bugs this would help prevent? > > No, but it does make me more confident about omitting calls to an > external dirname command, which should speed stuff up. I see. I suppose it comes down to whether not running dirname speeds things up enough to justify it. >>> Signed-off-by: Demi Marie Obenour >>> --- >>> scripts/make-erofs.sh | 31 +++++++++++++++++++++++++++++++ >>> 1 file changed, 31 insertions(+) >>> >>> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh >>> index e63bcbed9c3028f0f2b55431d46ba9ec67bc26ef..cf942972910c76e1835dc5b= 0084c2d04bf084a9d 100755 >>> --- a/scripts/make-erofs.sh >>> +++ b/scripts/make-erofs.sh >>> @@ -28,6 +28,34 @@ trap 'chmod -R +w -- "$root" && rm -rf -- "$superroo= t"' EXIT >>> root=3D$superroot/real_root >>> mkdir -- "$root" >>>=20=20 >>> +check_path () { >>> + # Various code can only handle paths that do not end with / >>> + # and are in canonical form. Reject others. >>> + for i; do >>> + case $i in >>> + (''|.|..|./*|../*|*/|*/.|*/..|*//*|*/./*|*/../*) >>> + printf 'Path "%s" is /, //, empty, or not canonical\n' "$i" >&2 >>> + exit 1 >>> + ;; >>> + (*[!A-Za-z0-9._@+/-]*) >>> + printf 'Path "%s" has forbidden characters\n' "$i" >&2 >>> + exit 1 >>> + ;; >>=20 >> Not sure why we'd want to rule out most characters? We're not really in >> control of what characters packages choose to use in their store paths. > > I believe Nix has an allowlist of permitted characters in store paths. > Is this documented, or is it just in the C++ source code? I'm not sure! I've not heard of such a thing. >>> + (-*) >>> + printf 'Path "%s" begins with -\n' "$i" >&2 >>> + exit 1 >>> + ;; >>> + (/nix/store/*|[!/]*) >>=20 >> It's technically possible to use Nix with a different store path, so I'd >> like to avoid anything that requires us to hardcode /nix/store. > > Right now, the generated images depend on the store paths, so > the scripts would need to be adapted to support this. If we > are going to generalize this, I recommend using a proper > scripting language like Python, Perl, or Lua. The only place I see where we hardcode a store path is host/initramfs/default.nix, which is a bug and easy to fix with Nix code. Of course you wouldn't reproduce the same image if you built with a different store directory, but it shouldn't be invalid to do so. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQRV/neXydHjZma5XLJbRZGEIw/wogUCaMHJdgAKCRBbRZGEIw/w orrwAQCAY3g6o8CNp+hLBTxZqGo5zbZGMvzry1i3NhIe6mvqhwD/b2y7Xse9VtXB 7cWg4ySvQE3XLgWXAwYiDnjmkG0KsA4= =wvvC -----END PGP SIGNATURE----- --=-=-=--