On Thu, Jun 20, 2024 at 02:12:53PM +0200, Stefano Brivio wrote:On Thu, 20 Jun 2024 12:30:54 +0100 "Richard W.M. Jones" <rjones(a)redhat.com> wrote:While I was testing this, I found we do seem to check it: https://passt.top/passt/tree/conf.c#n1446 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.orgOn Wed, May 29, 2024 at 12:35:24PM +1000, David Gibson wrote:Yes, please, that, or a patch :)On Wed, May 22, 2024 at 10:59:10PM +0200, Stefano Brivio wrote:Yes, agreed. It seems as if the latest passt code still does this. Do you want me to open a bug about it?Otherwise, if the user runs us as root, and gives us paths that are only accessible by root, we'll fail to open them, which might in turn encourage users to change permissions or ownerships: definitely a bad idea in terms of security. Reported-by: Minxi Hou <mhou(a)redhat.com> Reported-by: Richard W.M. Jones <rjones(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>Looking at this I did notice a pre-existing, well, maybe not bug exactly, but possibly surprising behaviour, which makes me a bit more nervous now that we can invoke it as root. tap_sock_unix_open() will silently truncate the given socket path to the maximum length for a Unix socket. Which means we could bind(), but also unlink() a path that's not exactly the same as the one the one the user requested. I don't immediately see a way to exploit that, but it's the sort of thing that makes me nervous. I think we should instead outright fail if the given socket path is too long.