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 19E101B1E2; Sat, 01 Nov 2025 12:17:32 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 52C681B1DD; Sat, 01 Nov 2025 12:17:30 +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-b6-smtp.messagingengine.com (fout-b6-smtp.messagingengine.com [202.12.124.149]) by atuin.qyliss.net (Postfix) with ESMTPS id 1237A1B1D5 for ; Sat, 01 Nov 2025 12:17:29 +0000 (UTC) Received: from phl-compute-06.internal (phl-compute-06.internal [10.202.2.46]) by mailfout.stl.internal (Postfix) with ESMTP id D5C261D001A3; Sat, 1 Nov 2025 08:17:27 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-06.internal (MEProxy); Sat, 01 Nov 2025 08:17:27 -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=fm2; t=1761999447; x=1762085847; bh=edAkkURREv hfNODFDejMJFsx6zKjKUv6T0/7MqyKAYc=; b=mcBZ+A/x5ARJO2Ru1tqODiKoSn Jk7nxYHRa0e/YdfSZXUAWfK/YvjGDRLq/H9mJju0Ozkn4UqC6O+zv2bE1/proByY uOV9dRCjGPLNpWDrCiUCx1Qc7QefLEjqZsPXXlupG+MLNoPbHEZbu6BKgRFOIJFo mGrDjERHVnUAEulyz6k05g+Qsxp7AXvl+HywRn015tKxMEL4osZCnkdrpFeZS8C8 IBCObwW/COOxlqlHfbfabTtF4AAK+qrIm1tR8aWJN4IdmGn38aiWP9bd/KE2Wd5q 2ccUZxYHUQQfxpSiqE+agOsjitc0yqq1wUOfYH/b2YuM7Tbvdgavrfq1Qj7g== 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=fm3; t= 1761999447; x=1762085847; bh=edAkkURREvhfNODFDejMJFsx6zKjKUv6T0/ 7MqyKAYc=; b=IaAosPJ02Cng2qEKjrAyixQQCKhxULsrOBWcwaFoufdL6KOAmHu SPq/u+whoFXfmo07wydgMj1zOAisvwFy4hUXrUKoLNDkFkvrjNhct9NcQAgctDsR NJeKnLYTv/FduuxRI2NkeORB2isPNM8VrtqidUG8GtbVwGp4LNm/OZxOhqcpXt+h 7OYgX2WN9spv+TdKxxtgYUZJ+tY078CSXsNW9z1OlZq+TobjgeRGIPd0fY44lmdA h037gUK4fiy4AdB5EjB42Xoneq+t4Vasv6m00HwpPQS4sC87vsq3SB4cuwgIwL3w tJCrheCa8tkZUbseHi/wQ242W67WudCxELg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggddujedvfeehucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhephffvvefujghffffkgggtsehgtderredttdejnecuhfhrohhmpeetlhihshhsrgcu tfhoshhsuceohhhisegrlhihshhsrgdrihhsqeenucggtffrrghtthgvrhhnpeetheevud fgjefghefhieejudelkeeljeegvdekueeuhffhgedvveefteevgeetieenucevlhhushht vghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehhihesrghlhihsshgrrd hishdpnhgspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohep uggvmhhiohgsvghnohhurhesghhmrghilhdrtghomhdprhgtphhtthhopeguvghvvghlse hsphgvtghtrhhumhdqohhsrdhorhhg X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sat, 1 Nov 2025 08:17:27 -0400 (EDT) Received: by mbp.qyliss.net (Postfix, from userid 1000) id 66E8662B5EB6; Sat, 01 Nov 2025 13:17:26 +0100 (CET) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH 3/7] tools: Add directory checker for updates In-Reply-To: <72921587-e951-4bfb-b68e-5cb05fc32609@gmail.com> References: <20251029-updates-v1-0-401c1be2a11b@gmail.com> <20251029-updates-v1-3-401c1be2a11b@gmail.com> <87sef1kjbk.fsf@alyssa.is> <72921587-e951-4bfb-b68e-5cb05fc32609@gmail.com> Date: Sat, 01 Nov 2025 13:17:25 +0100 Message-ID: <87bjlmq756.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: WASFL4XKQQG2LNV7EIARX4RPHUEEY6WO X-Message-ID-Hash: WASFL4XKQQG2LNV7EIARX4RPHUEEY6WO 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 10/29/25 08:01, Alyssa Ross wrote: >> Demi Marie Obenour writes: >>=20 >>> Spectrum OS's host has no network access. Updates must be downloaded by >>> VMs. The downloads are placed into a bind-mounted directory. The VM >>> can write whatever it wants into that directory. This includes symlinks >>> that subsequent code might open, which would create a path traversal >>> vulnerability. It also includes paths with names containing containing >>> terminal escape sequences, newlines, or other nastiness. Furthermore, >>> the directory should not have any subdirectories either. >>> >>> Add a simple C program that checks for such ugliness and indicates >>> (via its exit code) if the VM misbehaved. It also ensures that both >>> SHA256SUMS and SHA256SUMS.gpg are present. >>> >>> Signed-off-by: Demi Marie Obenour >>> --- >>> host/rootfs/Makefile | 6 +- >>> lib/kcmdline-utils.mk | 6 ++ >>> tools/default.nix | 1 + >>> tools/meson.build | 1 + >>> tools/updates-dir-check/meson.build | 4 ++ >>> tools/updates-dir-check/updates-dir-check.c | 94 +++++++++++++++++++++= ++++++++ >>> 6 files changed, 110 insertions(+), 2 deletions(-) >>=20 >> I still don't really understand why this needs to be a C program instead >> of find -H /path/to/dir -not -type f. None of the other checks seem >> very necessary? > > I trust this code more than I trust (especially) the Busybox > implementation of find. This doesn't really make sense to me. All of this is quite trivial find behaviour =E2=80=94 not the sort of thing that's unlikely to have been wide= ly tested. No objection to GNU find though if it helps. >> How are -Werror=3Dpedantic and -DNDEBUG getting enabled in the first pla= ce? > > I believe Meson sets -DNDEBUG in some cases. Yes, if the user explicitly asks for it. >>> + if (entry->d_name[0] =3D=3D '.') >>> + if (len =3D=3D 1 || (len =3D=3D 2 && entry->d_name[1] =3D=3D '.')) >>> + continue; >>> + if (strcmp(entry->d_name, "SHA256SUMS") =3D=3D 0) { >>> + found_sha256sums =3D true; >>> + continue; >>> + } >>> + if (strcmp(entry->d_name, "SHA256SUMS.gpg") =3D=3D 0) { >>> + found_sha256sums_gpg =3D true; >>> + continue; >>> + } >>> + unsigned char c =3D (unsigned char)entry->d_name[0]; >>> + if (!((c >=3D 'A' && c <=3D 'Z') || >>> + (c >=3D 'a' && c <=3D 'z'))) >>> + errx(1, "Filename must begin with an ASCII letter"); >>> + for (size_t i =3D 1; i < len; ++i) { >>> + c =3D (unsigned char)entry->d_name[i]; >>> + if (!((c >=3D 'A' && c <=3D 'Z') || >>> + (c >=3D 'a' && c <=3D 'z') || >>> + (c >=3D '0' && c <=3D '9') || >>> + (c =3D=3D '_') || >>> + (c =3D=3D '-') || >>> + (c =3D=3D '.'))) { >>> + if (c >=3D 0x20 && c <=3D 0x7E) >>> + errx(1, "Forbidden subsequent character in filename: '%c'", (int)= c); >>> + else >>> + errx(1, "Forbidden subsequent character in filename: byte %d", (i= nt)c); >>> + } >>> + } >>=20 >> Why do we care? Surely we don't expect systemd-sysupdate to put >> filenames unescaped into a shell or something. > > Prevent escape sequence injection into terminals and logs is the > main reason. Qubes OS has similar checks in some places, though they > are off by default for file copying. Doing this in a tool that's only used by sysupdate is a very ad-hoc way to protect against that. I think if we want to protect against that sort of thing it should be done in one place, probably in virtiofsd. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQRV/neXydHjZma5XLJbRZGEIw/wogUCaQX6VQAKCRBbRZGEIw/w om8lAQDrPXs3jcWc0TdJ72Y9McvrwBGAiUtBU831bHPHXQkVtQD/VmiCorELkz9l S6akcv3IFKXO/yOdDFwFcj/cxJNWnQk= =EQal -----END PGP SIGNATURE----- --=-=-=--