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 E277A215B5; Sun, 02 Nov 2025 12:18:09 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id DFEBF21649; Sun, 02 Nov 2025 12:18:07 +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-b5-smtp.messagingengine.com (fout-b5-smtp.messagingengine.com [202.12.124.148]) by atuin.qyliss.net (Postfix) with ESMTPS id AF30321647 for ; Sun, 02 Nov 2025 12:18:06 +0000 (UTC) Received: from phl-compute-01.internal (phl-compute-01.internal [10.202.2.41]) by mailfout.stl.internal (Postfix) with ESMTP id 3A6141D00114; Sun, 2 Nov 2025 07:18:05 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Sun, 02 Nov 2025 07:18:05 -0500 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=1762085885; x=1762172285; bh=M8j/l6M02Q Xs+KCwrUy0y8O1XGaeYlCQYICqtBm75Ko=; b=eIu3r6JpLsI6rhsnfUkRWbu0QG ELNd9aw1smHG4RUKsvhLUYUurGANj/obMVdgCKr7a2TzD/i2PD1F87iJQDpJtyaV MpPX6jEG/bp2OqzfD1eGfdLb7gwjTo9dxjv7PETpuw83Sxv2hEAH5mZY9B/D5ENn CleIZZ3jtR/WbMpYNnFHBr6vGTw0ScofTRzauXsAYGEM/napecXANt6cSbc0hHdz QbnweQ3g5vil7jZcyFx7i8B1cBMicVVyFLItNCErIA/8zTJzxXNnHFZxRkkR3HMB Ih7Yq5TLq56UMkEhL20LU95x22MzJu6+pJw6ZvPacN5RpHMDFCyNygHcHhgw== 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= 1762085885; x=1762172285; bh=M8j/l6M02QXs+KCwrUy0y8O1XGaeYlCQYIC qtBm75Ko=; b=gIYynwIOPXqS0Vr8HbOjEZpB/arZNz/qx+2dgdoLdqWMfJnxryr Uyo4PFD1+KAYr4UVNGBtk+AYuqvQey8smNVpHfYUcaNnIq3daRi6rS5AazHKc+Az ioGtGhcqnNPVm0z7mzzCpuRn/U/+6v9K+074H6hQMXaQ7S8BCmVybjwnk8adsYdL 5FkfXrh1bVOZ3tC8Cx7S6CfeZRCluru3wvHpsQdNsfqNICtfr0iKfotAfg75toaK YhlAkZP30h850gEVqzGcwcZRd6IYbXrx0V1JUvQn/MfOCX64V+ogmIlwwaE2EK0T uNLI+NkvICmnONK5tvnqiJmaAQsu4w0ZD+g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggddujeehvdefucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhephffvvefujghffffkgggtsehgtderredttdejnecuhfhrohhmpeetlhihshhsrgcu tfhoshhsuceohhhisegrlhihshhsrgdrihhsqeenucggtffrrghtthgvrhhnpeegvdffke fhtedvjeevtdfgudfguddtveegjeetiefgvddtkeelgeefueeugeehgeenucffohhmrghi nhepghhithhhuhgsrdgtohhmpdgtohhmphhilhgvrhhsrdhphienucevlhhushhtvghruf hiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehhihesrghlhihsshgrrdhishdp nhgspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohepuggvmh hiohgsvghnohhurhesghhmrghilhdrtghomhdprhgtphhtthhopeguvghvvghlsehsphgv tghtrhhumhdqohhsrdhorhhg X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sun, 2 Nov 2025 07:18:04 -0500 (EST) Received: by mbp.qyliss.net (Postfix, from userid 1000) id AE25463ABA12; Sun, 02 Nov 2025 13:18:02 +0100 (CET) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH 3/7] tools: Add directory checker for updates In-Reply-To: <831ecec1-d782-4fab-a6d5-40eae0f9ad92@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> <87bjlmq756.fsf@alyssa.is> <831ecec1-d782-4fab-a6d5-40eae0f9ad92@gmail.com> Date: Sun, 02 Nov 2025 13:18:02 +0100 Message-ID: <87ms54pr0l.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: EUDFM735EK7CBI43RCWHTPCT2STNWIQJ X-Message-ID-Hash: EUDFM735EK7CBI43RCWHTPCT2STNWIQJ 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 11/1/25 08:17, Alyssa Ross wrote: >> Demi Marie Obenour writes: >>=20 >>> On 10/29/25 08:01, Alyssa Ross wrote: >>>> Demi Marie Obenour writes: >>>> >>>>> 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 symli= nks >>>>> that subsequent code might open, which would create a path traversal >>>>> vulnerability. It also includes paths with names containing containi= ng >>>>> 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(-) >>>> >>>> I still don't really understand why this needs to be a C program inste= ad >>>> 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. >>=20 >> 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 w= idely >> tested. No objection to GNU find though if it helps. > > I see: find with a -exec false to return an error if anything matching > is found? > > I'm way more familiar with C than with find, which is why I missed this. Hmm, thinking about it some more I suppose there's a problem with find: there's no way to get it to exit as soon as it finds a matching file, with a failing error code, so it could end up running way too long. So the C program is fine, I guess. >>>> How are -Werror=3Dpedantic and -DNDEBUG getting enabled in the first p= lace? >>> >>> I believe Meson sets -DNDEBUG in some cases. >>=20 >> Yes, if the user explicitly asks for it. > > I thought it was default for release builds. Doesn't look like it: https://github.com/mesonbuild/meson/blob/d00f840c573103c2d51aed2b169386f7ac= fe7026/mesonbuild/compilers/compilers.py#L255-L264 b_ndebug defaults to false. >>>>> + 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'", (in= t)c); >>>>> + else >>>>> + errx(1, "Forbidden subsequent character in filename: byte %d", = (int)c); >>>>> + } >>>>> + } >>>> >>>> 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. >>=20 >> 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. > > I think sysupdate is more likely to log unsanitized data, especially > as systemd-journald has no problems with it. What's the difference between systemd-journald's behaviour and the logging we have? --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQRV/neXydHjZma5XLJbRZGEIw/wogUCaQdL+gAKCRBbRZGEIw/w oqYZAQCFif0mo4pf6WTpJGGFHnooCwLf/h/cle6fno4gRvIDRQEAvm9HO/jl3PN2 CzVF0UFSQjQQHdo5MOREGkbcERoMRQ4= =lYZV -----END PGP SIGNATURE----- --=-=-=--