diff --git a/src/pxar/encoder.rs b/src/pxar/encoder.rs index 491bee2e..c92fd183 100644 --- a/src/pxar/encoder.rs +++ b/src/pxar/encoder.rs @@ -230,25 +230,29 @@ impl <'a, W: Write> Encoder<'a, W> { (self.feature_flags & feature_flags) == feature_flags } - fn read_xattrs(&self, fd: RawFd, stat: &FileStat, entry: &CaFormatEntry) -> Result<(Vec, Option), Error> { + fn read_xattrs(&self, fd: RawFd, stat: &FileStat) -> Result<(Vec, Option), Error> { let mut xattrs = Vec::new(); let mut fcaps = None; let flags = CA_FORMAT_WITH_XATTRS | CA_FORMAT_WITH_FCAPS; - if !self.has_features(flags) { return Ok((xattrs, fcaps)); } + if !self.has_features(flags) { + return Ok((xattrs, fcaps)); + } // Should never be called on symlinks, just in case check anyway - if (stat.st_mode & libc::S_IFMT) == libc::S_IFLNK { return Ok((xattrs, fcaps)); } + if (stat.st_mode & libc::S_IFMT) == libc::S_IFLNK { + return Ok((xattrs, fcaps)); + } let xattr_names = match xattr::flistxattr(fd) { Ok(names) => names, - Err(Errno::EOPNOTSUPP) => return Ok((xattrs, fcaps)), - Err(Errno::EBADF) => return Ok((xattrs, fcaps)), Err(err) => bail!("read_xattrs failed for {:?} - {}", self.full_path(), err), }; - for name in xattr_names.split(|c| *c == '\0' as u8) { + for name in xattr_names.split(|c| *c == b'\0') { // Only extract the relevant extended attributes - if !xattr::name_store(&name) { continue; } + if !xattr::is_valid_xattr_name(&name) { + continue; + } let value = match xattr::fgetxattr(fd, name) { Ok(value) => value, @@ -257,7 +261,7 @@ impl <'a, W: Write> Encoder<'a, W> { Err(err) => bail!("read_xattrs failed for {:?} - {}", self.full_path(), err), }; - if xattr::security_capability(&name) { + if xattr::is_security_capability(&name) { // fcaps are stored in own format within the archive fcaps = Some(CaFormatFCaps { data: value, @@ -355,10 +359,12 @@ impl <'a, W: Write> Encoder<'a, W> { self.read_chattr(rawfd, &mut dir_entry)?; self.read_fat_attr(rawfd, magic, &mut dir_entry)?; - let (xattrs, fcaps) = self.read_xattrs(rawfd, &dir_stat, &dir_entry)?; + let (xattrs, fcaps) = self.read_xattrs(rawfd, &dir_stat)?; self.write_entry(dir_entry)?; - for xattr in xattrs { self.write_xattr(xattr)?; } + for xattr in xattrs { + self.write_xattr(xattr)?; + } self.write_fcaps(fcaps)?; let mut dir_count = 0; @@ -545,10 +551,12 @@ impl <'a, W: Write> Encoder<'a, W> { self.read_chattr(filefd, &mut entry)?; self.read_fat_attr(filefd, magic, &mut entry)?; - let (xattrs, fcaps) = self.read_xattrs(filefd, &stat, &entry)?; + let (xattrs, fcaps) = self.read_xattrs(filefd, &stat)?; self.write_entry(entry)?; - for xattr in xattrs { self.write_xattr(xattr)?; } + for xattr in xattrs { + self.write_xattr(xattr)?; + } self.write_fcaps(fcaps)?; let include_payload; diff --git a/src/pxar/sequential_decoder.rs b/src/pxar/sequential_decoder.rs index 785826bc..51b2a399 100644 --- a/src/pxar/sequential_decoder.rs +++ b/src/pxar/sequential_decoder.rs @@ -150,25 +150,20 @@ impl <'a, R: Read> SequentialDecoder<'a, R> { fn read_xattr(&mut self, size: usize) -> Result { let buffer = self.reader.read_exact_allocated(size)?; - match buffer.iter().position(|c| *c == '\0' as u8) { - Some(pos) => { - // pos needs to be within the first 256 bytes in order to - // terminate a valid xattr name - if pos > 255 { - bail!("Invalid zero termination for xattr name."); - } - if !xattr::name_store(&buffer[0..pos]) || xattr::security_capability(&buffer[0..pos]) { - bail!("Invalid name for xattr - {}.", String::from_utf8_lossy(&buffer[0..pos])); - } - let name = buffer[0..pos].to_vec(); - let value = buffer[pos + 1..size].to_vec(); - return Ok(CaFormatXAttr { - name: name, - value: value, - }); - }, - _ => bail!("Incorrect zero termination in xattr."), + let separator = buffer.iter().position(|c| *c == b'\0') + .ok_or_else(|| format_err!("no value found in xattr"))?; + + let (name, value) = buffer.split_at(separator); + if !xattr::is_valid_xattr_name(name) || + xattr::is_security_capability(name) + { + bail!("incorrect xattr name - {}.", String::from_utf8_lossy(name)); } + + Ok(CaFormatXAttr { + name: name.to_vec(), + value: value[1..].to_vec(), + }) } fn read_fcaps(&mut self, size: usize) -> Result { diff --git a/src/tools/xattr.rs b/src/tools/xattr.rs index f472b7a3..caa49312 100644 --- a/src/tools/xattr.rs +++ b/src/tools/xattr.rs @@ -92,17 +92,20 @@ pub fn fsetxattr_fcaps(fd: RawFd, fcaps: CaFormatFCaps) -> Result<(), nix::errno Ok(()) } -pub fn security_capability(name: &[u8]) -> bool { +pub fn is_security_capability(name: &[u8]) -> bool { name == b"security.capability" } -pub fn name_store(name: &[u8]) -> bool { - if name.is_empty() { return false; } - if name.starts_with(b"user.") { return true; } - if name.starts_with(b"trusted.") { return true; } - if security_capability(name) { return true; } - - false +/// Check if the passed name buffer starts with a valid xattr namespace prefix +/// and is within the length limit of 255 bytes +pub fn is_valid_xattr_name(name: &[u8]) -> bool { + if name.is_empty() || name.len() > 255 { + return false; + } + if name.starts_with(b"user.") || name.starts_with(b"trusted.") { + return true; + } + is_security_capability(name) } #[cfg(test)]