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 1E2486F01; Tue, 04 Nov 2025 15:27:47 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id AD63F6EEB; Tue, 04 Nov 2025 15:27:43 +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 fhigh-a8-smtp.messagingengine.com (fhigh-a8-smtp.messagingengine.com [103.168.172.159]) by atuin.qyliss.net (Postfix) with ESMTPS id 986986EE9 for ; Tue, 04 Nov 2025 15:27:41 +0000 (UTC) Received: from phl-compute-04.internal (phl-compute-04.internal [10.202.2.44]) by mailfhigh.phl.internal (Postfix) with ESMTP id A9FE114001EC; Tue, 4 Nov 2025 10:27:40 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-04.internal (MEProxy); Tue, 04 Nov 2025 10:27:40 -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=1762270060; x=1762356460; bh=l9F3AeQitV yVHwLNYozdACziRKJqWtMgOXauiinYmbI=; b=SQdWX2a2kbiLIWb8aM/inHtHKj VeU/LnGkiwtI41FjrZKgnZ4pL8TcK1II0fBUTb8AujYzqObQHciPWR1CDwx84y+4 cvwbYiD58HhEZIU12KvGkXMXd5JKWHUgouORqZVBH/aoJt0wttmRmfw8W4GwlZxp PzaKgHLx0/BJIvfjWzV+y77ONpW79kn0PjsaQzBYPPaAP7Z63PLvfAw6IGnQ0E+5 ZxC645SzGxaqC9T8T+5TMAohDEV1ljZ+x2i6mUu8DWA4Grr4KnTI7rJ+aPRlEkrx ypiZnv1nIEpQqymlWLQTmza8eCIi7ezzf3YpGNWTOzJcgE2TZ2cyfXX/Opfg== 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= 1762270060; x=1762356460; bh=l9F3AeQitVyVHwLNYozdACziRKJqWtMgOXa uiinYmbI=; b=sYHdjxj3HJMmDBdV681pfrtJrVpJ9AkrC3IU/LZYw6VWyaTh9eV O0yB3GaHmKBnhFizvcQhN03Iui7BHi1I/YJduhb/FOdIZpyolcXIkWmyn5huD6Li jz/tGcU/emeH6ktavTsn7SzbEB7MvlrnumcSG27pD+gz5/uByC8OBRgoeOPFkSNY QXW/kqaSDzlvslYd+Ryk0sBqe3ynxAxW+CeZekylc/kJDBoORTXnddqzB7xeR2cq 296FvMaiNCxIINMQsllen3lz0NESrWO3HFXKprMCs2i9K6CjZeaQ2fUgCxnY23CH VukYCHYyg2XGn1nITyXOqjrPXBO13gXJQxg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggddukedufeekucetufdoteggodetrf 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; Tue, 4 Nov 2025 10:27:40 -0500 (EST) Received: by mbp.qyliss.net (Postfix, from userid 1000) id 506BF642BC07; Tue, 04 Nov 2025 16:27:39 +0100 (CET) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH 3/7] tools: Add directory checker for updates In-Reply-To: <9296ee2d-8e8d-4579-b0f0-638d9e0583d8@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> <87ms54pr0l.fsf@alyssa.is> <9296ee2d-8e8d-4579-b0f0-638d9e0583d8@gmail.com> Date: Tue, 04 Nov 2025 16:27:38 +0100 Message-ID: <87h5v9rf6d.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: 7J2XUWUOMO5OZZOUTUHMO3N3ASVJWRNO X-Message-ID-Hash: 7J2XUWUOMO5OZZOUTUHMO3N3ASVJWRNO 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/2/25 07:18, Alyssa Ross wrote: >> Demi Marie Obenour writes: >>=20 >>> On 11/1/25 08:17, Alyssa Ross wrote: >>>> Demi Marie Obenour writes: >>>> >>>>> On 10/29/25 08:01, Alyssa Ross wrote: >>>>>> Demi Marie Obenour writes: >>>>>> >>>>>>> Spectrum OS's host has no network access. Updates must be download= ed by >>>>>>> VMs. The downloads are placed into a bind-mounted directory. The = VM >>>>>>> can write whatever it wants into that directory. This includes sym= links >>>>>>> that subsequent code might open, which would create a path traversal >>>>>>> vulnerability. It also includes paths with names containing contai= ning >>>>>>> terminal escape sequences, newlines, or other nastiness. Furthermo= re, >>>>>>> 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 ins= tead >>>>>> 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 fi= nd >>>> behaviour =E2=80=94 not the sort of thing that's unlikely to have been= widely >>>> 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. >>=20 >> 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. >>=20 >> So the C program is fine, I guess. >>=20 >>>>>> How are -Werror=3Dpedantic and -DNDEBUG getting enabled in the first= place? >>>>> >>>>> I believe Meson sets -DNDEBUG in some cases. >>>> >>>> Yes, if the user explicitly asks for it. >>> >>> I thought it was default for release builds. >>=20 >> Doesn't look like it: >>=20 >> https://github.com/mesonbuild/meson/blob/d00f840c573103c2d51aed2b169386f= 7acfe7026/mesonbuild/compilers/compilers.py#L255-L264 >>=20 >> b_ndebug defaults to false. > > Got it, thanks! > >>>>>>> + 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"= , (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. >>>> >>>> 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. >>=20 >> What's the difference between systemd-journald's behaviour and the >> logging we have? > > I'm not familiar with s6 at all, but I think it is at least worth > investigating. Also, all else equal it is best to reject invalid > untrusted input as early as possible. As early as possible would be in virtiofsd, not ad-hoc for this one service here. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQRV/neXydHjZma5XLJbRZGEIw/wogUCaQobagAKCRBbRZGEIw/w oj03AP4kKBtN/tcltIAZXSMcrPVNKEUJF5Sv9YRcMUyHxGaz5wD/T0/KFubIR9DZ za7EZQj8VDoNb4PKRvP1aUModgSbWgM= =HSq4 -----END PGP SIGNATURE----- --=-=-=--