lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240826-indikatoren-fernverkehr-2208f3c38ddd@brauner>
Date: Mon, 26 Aug 2024 14:54:49 +0200
From: Christian Brauner <brauner@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Luis Chamberlain <mcgrof@...nel.org>, Jann Horn <jannh@...gle.com>, 
	Russ Weight <russ.weight@...ux.dev>, Danilo Krummrich <dakr@...hat.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2] firmware_loader: Block path traversal

On Sat, Aug 24, 2024 at 09:14:30AM GMT, Linus Torvalds wrote:
> On Sat, Aug 24, 2024 at 5:13 AM Luis Chamberlain <mcgrof@...nel.org> wrote:
> >
> > I'm all for this, however a strong rejection outright for the first
> > kernel release is bound to end up with some angry user with some oddball
> > driver that had this for whatever stupid reason.
> 
> I can't actually see a reason why a firmware file would have a ".."
> component in it, so I think the immediate rejection is fine -
> particularly since it has a warning printout, so you see what happened
> and why.
> 
> I do wonder if we should just have a LOOKUP_NO_DOTDOT flag, and just use that.
> 
> [ Christian - the issue is the firmware loading path not wanting to
> have ".." in the pathname so that you can't load outside the normal
> firmware tree. We could also use LOOKUP_BENEATH, except
> kernel_read_file_from_path_initns() just takes one long path rather
> than "here's the base, and here's the path". ]
> 
> There might be other people who want LOOKUP_NO_DOTDOT for similar
> reasons. In fact, some people might want an even stronger "normalized
> path" validation, where empty components or just "." is invalid, just
> because that makes pathnames ambiguous.

I think LOOKUP_NO_DOTDOT is potentially interesting for userspace as
RESOLVE_NO_DOTDOT. Though Jann's reply made me wonder: If some userspace
project wants to make sure that there are no ".." in the path while
allowing symlinks and ".." in symlinks then wouldn't it be easier and
cheaper for userspace to just manually check for ".." in the path
themselves? I mean RESOLVE_NO_SYMLINKS and RESOLVE_NO_MAGICLINKS
alleviate some proper pain. Not just because an appropariate userspace
algorithm is very costly but also because it's hard to get right. But
with RESOLVE_NO_DOTOT it feels like a pretty mundane string-parsing
excercise that could be done in userspace. Although I may just lack
imagination.

As long as we only have this very limited in-kernel user I think just
using Jann's helper in this patch is probably good enough.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ