diff --git a/src/devices/src/virtio/fs/linux/passthrough.rs b/src/devices/src/virtio/fs/linux/passthrough.rs index dc87749b9..6d5606c45 100644 --- a/src/devices/src/virtio/fs/linux/passthrough.rs +++ b/src/devices/src/virtio/fs/linux/passthrough.rs @@ -352,6 +352,14 @@ enum FileOrLink { Link(CString), } +fn sanitize_name_at(inode: Inode, name: &CStr) -> &CStr { + if inode == fuse::ROOT_ID && name == c".." { + c"." + } else { + name + } +} + impl PassthroughFs { pub fn new(cfg: Config) -> io::Result { let fd = if let Some(fd) = cfg.proc_sfd_rawfd { @@ -491,6 +499,7 @@ impl PassthroughFs { } fn do_lookup(&self, parent: Inode, name: &CStr) -> io::Result { + let name = sanitize_name_at(parent, name); let p = self .inodes .read() @@ -761,6 +770,8 @@ impl PassthroughFs { } fn do_unlink(&self, parent: Inode, name: &CStr, flags: libc::c_int) -> io::Result<()> { + let name = sanitize_name_at(parent, name); + let data = self .inodes .read() @@ -1003,6 +1014,7 @@ impl FileSystem for PassthroughFs { umask: u32, extensions: Extensions, ) -> io::Result { + let name = sanitize_name_at(parent, name); if extensions.secctx.is_some() { unimplemented!("SECURITY_CTX is not supported and should not be used by the guest"); } @@ -1026,6 +1038,7 @@ impl FileSystem for PassthroughFs { } fn rmdir(&self, _ctx: Context, parent: Inode, name: &CStr) -> io::Result<()> { + let name = sanitize_name_at(parent, name); self.do_unlink(parent, name, libc::AT_REMOVEDIR) } @@ -1106,6 +1119,7 @@ impl FileSystem for PassthroughFs { umask: u32, extensions: Extensions, ) -> io::Result<(Entry, Option, OpenOptions)> { + let name = sanitize_name_at(parent, name); if extensions.secctx.is_some() { unimplemented!("SECURITY_CTX is not supported and should not be used by the guest"); } @@ -1396,6 +1410,9 @@ impl FileSystem for PassthroughFs { newname: &CStr, flags: u32, ) -> io::Result<()> { + let oldname = sanitize_name_at(olddir, oldname); + let newname = sanitize_name_at(newdir, newname); + let old_inode = self .inodes .read() @@ -1441,6 +1458,7 @@ impl FileSystem for PassthroughFs { umask: u32, extensions: Extensions, ) -> io::Result { + let name = sanitize_name_at(parent, name); if extensions.secctx.is_some() { unimplemented!("SECURITY_CTX is not supported and should not be used by the guest"); } @@ -1478,6 +1496,8 @@ impl FileSystem for PassthroughFs { newparent: Inode, newname: &CStr, ) -> io::Result { + let newname = sanitize_name_at(newparent, newname); + let data = self .inodes .read() @@ -1521,6 +1541,8 @@ impl FileSystem for PassthroughFs { name: &CStr, extensions: Extensions, ) -> io::Result { + let name = sanitize_name_at(parent, name); + // Set security context on symlink. if extensions.secctx.is_some() { unimplemented!("SECURITY_CTX is not supported and should not be used by the guest"); @@ -1704,6 +1726,7 @@ impl FileSystem for PassthroughFs { value: &[u8], flags: u32, ) -> io::Result<()> { + let name = sanitize_name_at(inode, name); if !self.cfg.xattr { return Err(io::Error::from_raw_os_error(libc::ENOSYS)); } @@ -1752,6 +1775,7 @@ impl FileSystem for PassthroughFs { name: &CStr, size: u32, ) -> io::Result { + let name = sanitize_name_at(inode, name); if !self.cfg.xattr { return Err(io::Error::from_raw_os_error(libc::ENOSYS)); } @@ -1844,6 +1868,7 @@ impl FileSystem for PassthroughFs { } fn removexattr(&self, _ctx: Context, inode: Inode, name: &CStr) -> io::Result<()> { + let name = sanitize_name_at(inode, name); if !self.cfg.xattr { return Err(io::Error::from_raw_os_error(libc::ENOSYS)); } diff --git a/src/devices/src/virtio/fs/mod.rs b/src/devices/src/virtio/fs/mod.rs index ea475a5c1..bb66ebbe2 100644 --- a/src/devices/src/virtio/fs/mod.rs +++ b/src/devices/src/virtio/fs/mod.rs @@ -61,6 +61,8 @@ pub enum FsError { /// A C string parameter is invalid. InvalidCString(FromBytesWithNulError), InvalidCString2(FromVecWithNulError), + /// Given string is not a valid filename + InvalidFilename, /// The `len` field of the header is too small. InvalidHeaderLength, /// The `size` field of the `SetxattrIn` message does not match the length diff --git a/src/devices/src/virtio/fs/server.rs b/src/devices/src/virtio/fs/server.rs index 0d96a60cc..a76f93400 100644 --- a/src/devices/src/virtio/fs/server.rs +++ b/src/devices/src/virtio/fs/server.rs @@ -192,7 +192,7 @@ impl Server { r.read_exact(&mut buf).map_err(Error::DecodeMessage)?; - let name = bytes_to_cstr(buf.as_ref())?; + let name = filename_from_bytes(buf.as_ref())?; match self .fs @@ -310,9 +310,11 @@ impl Server { match self.fs.symlink( Context::from(in_header), + // Target of the symlink can be a path - it can contain '/' bytes_to_cstr(linkname)?, in_header.nodeid.into(), - bytes_to_cstr(name)?, + // The symlink cannot contain '/' + filename_from_bytes(name)?, extensions, ) { Ok(entry) => { @@ -346,7 +348,7 @@ impl Server { match self.fs.mknod( Context::from(in_header), in_header.nodeid.into(), - bytes_to_cstr(name)?, + filename_from_bytes(name)?, mode, rdev, umask, @@ -381,7 +383,7 @@ impl Server { match self.fs.mkdir( Context::from(in_header), in_header.nodeid.into(), - bytes_to_cstr(name)?, + filename_from_bytes(name)?, mode, umask, extensions, @@ -406,7 +408,7 @@ impl Server { match self.fs.unlink( Context::from(in_header), in_header.nodeid.into(), - bytes_to_cstr(&name)?, + filename_from_bytes(&name)?, ) { Ok(()) => reply_ok(None::, None, in_header.unique, w), Err(e) => reply_error(e, in_header.unique, w), @@ -424,7 +426,7 @@ impl Server { match self.fs.rmdir( Context::from(in_header), in_header.nodeid.into(), - bytes_to_cstr(&name)?, + filename_from_bytes(&name)?, ) { Ok(()) => reply_ok(None::, None, in_header.unique, w), Err(e) => reply_error(e, in_header.unique, w), @@ -460,9 +462,9 @@ impl Server { match self.fs.rename( Context::from(in_header), in_header.nodeid.into(), - bytes_to_cstr(oldname)?, + filename_from_bytes(oldname)?, newdir.into(), - bytes_to_cstr(newname)?, + filename_from_bytes(newname)?, flags, ) { Ok(()) => reply_ok(None::, None, in_header.unique, w), @@ -500,7 +502,7 @@ impl Server { Context::from(in_header), oldnodeid.into(), in_header.nodeid.into(), - bytes_to_cstr(&name)?, + filename_from_bytes(&name)?, ) { Ok(entry) => { let out = EntryOut::from(entry); @@ -755,7 +757,7 @@ impl Server { match self.fs.getxattr( Context::from(in_header), in_header.nodeid.into(), - bytes_to_cstr(&name)?, + filename_from_bytes(&name)?, size, ) { Ok(GetxattrReply::Value(val)) => reply_ok(None::, Some(&val), in_header.unique, w), @@ -1115,7 +1117,7 @@ impl Server { match self.fs.create( Context::from(in_header), in_header.nodeid.into(), - bytes_to_cstr(name)?, + filename_from_bytes(name)?, mode, flags, umask, @@ -1475,6 +1477,14 @@ fn bytes_to_cstr(buf: &[u8]) -> Result<&CStr> { CStr::from_bytes_with_nul(buf).map_err(Error::InvalidCString) } +fn filename_from_bytes(buf: &[u8]) -> Result<&CStr> { + let cstr = bytes_to_cstr(buf)?; + if cstr.to_bytes().contains(&b'/') { + return Err(Error::InvalidFilename); + } + Ok(cstr) +} + fn add_dirent( cursor: &mut Writer, max: u32,