Skip to content

Commit

Permalink
Perform checked arithmetic as a defensive measure.
Browse files Browse the repository at this point in the history
This also bumps the version of `goblin` to at least 0.8.2, which allows
us to clean up some code.
  • Loading branch information
afranchuk committed Aug 26, 2024
1 parent 5f322e8 commit 21b0e13
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 13 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ thiserror = "1.0"

[target.'cfg(unix)'.dependencies]
libc = "0.2"
goblin = "0.8"
goblin = "0.8.2"
memmap2 = "0.9"

[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
Expand Down
3 changes: 2 additions & 1 deletion src/linux/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,11 @@ pub enum WriterError {

#[derive(Debug, Error)]
pub enum ModuleReaderError {
#[error("failed to read module memory: {length} bytes at {offset}: {error}")]
#[error("failed to read module memory: {length} bytes at {offset}{}: {error}", .start_address.map(|addr| format!(" (start address: {addr})")).unwrap_or_default())]
ReadModuleMemory {
offset: u64,
length: u64,
start_address: Option<u64>,
#[source]
error: nix::Error,
},
Expand Down
54 changes: 43 additions & 11 deletions src/linux/module_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,28 +49,48 @@ impl<'buf> ProcessMemory<'buf> {
Self::Process(pr) => {
let len = std::num::NonZero::new(length as usize).ok_or_else(|| {
Error::ReadModuleMemory {
start_address: Some(pr.start_address),
offset,
length,
error: nix::errno::Errno::EINVAL,
}
})?;
let proc_offset = pr.start_address.checked_add(offset).ok_or_else(|| {
Error::ReadModuleMemory {
start_address: Some(pr.start_address),
offset,
length,
error: nix::Error::EOVERFLOW,
}
})?;
pr.inner
.read_to_vec((pr.start_address + offset) as _, len)
.read_to_vec(proc_offset as _, len)
.map(Cow::Owned)
.map_err(|err| Error::ReadModuleMemory {
start_address: Some(pr.start_address),
offset,
length,
error: err.source,
})
}
Self::Slice(s) => s
.get(offset as usize..(offset + length) as usize)
.map(Cow::Borrowed)
.ok_or_else(|| Error::ReadModuleMemory {
offset,
length,
error: nix::Error::EACCES,
}),
Self::Slice(s) => {
let end = offset
.checked_add(length)
.ok_or_else(|| Error::ReadModuleMemory {
start_address: None,
offset,
length,
error: nix::Error::EOVERFLOW,
})?;
s.get(offset as usize..end as usize)
.map(Cow::Borrowed)
.ok_or_else(|| Error::ReadModuleMemory {
start_address: None,
offset,
length,
error: nix::Error::EACCES,
})
}
}
}

Expand Down Expand Up @@ -141,7 +161,7 @@ fn section_header_with_name<'sc>(
Ok(None)
}

/// Types which can be read from an `impl ModuleMemory`.
/// Types which can be read from ProcessMemory.
pub trait ReadFromModule: Sized {
fn read_from_module(module_memory: ProcessMemory<'_>) -> Result<Self, Error>;
}
Expand Down Expand Up @@ -279,7 +299,12 @@ impl<'buf> ModuleReader<'buf> {
(_, _, None) => Err(Error::NoSoNameEntry),
(Some(addr), Some(size), Some(offset)) => {
// If loaded in memory, the address will be altered to be absolute.
self.read_name_from_strtab(self.module_memory.absolute(addr), size, offset)
if offset < size {
self.read_name_from_strtab(self.module_memory.absolute(addr), size, offset)
} else {
log::warn!("soname strtab offset ({offset}) exceeds strtab size ({size})");
Err(Error::NoSoNameEntry)
}
}
}
}
Expand Down Expand Up @@ -320,6 +345,11 @@ impl<'buf> ModuleReader<'buf> {
dynstr_section_header.sh_size,
name_offset,
);
} else {
log::warn!(
"soname offset ({name_offset}) exceeds dynstr section size ({})",
dynstr_section_header.sh_size
);
}
}
}
Expand Down Expand Up @@ -394,6 +424,7 @@ impl<'buf> ModuleReader<'buf> {
strtab_size: u64,
name_offset: u64,
) -> Result<String, Error> {
assert!(name_offset < strtab_size);
let name = self
.module_memory
.read(strtab_offset + name_offset, strtab_size - name_offset)?;
Expand Down Expand Up @@ -436,6 +467,7 @@ impl<'buf> ModuleReader<'buf> {
self.header.e_shoff,
self.header.e_shentsize as u64 * self.header.e_shnum as u64,
)?;
// Use `parse_from` rather than `parse`, which allows a 0 offset.
let section_headers = elf::SectionHeader::parse_from(
&section_headers_data,
0,
Expand Down

0 comments on commit 21b0e13

Please sign in to comment.