Skip to content

Commit

Permalink
elf: fail validation if writable .data sections are present (#240)
Browse files Browse the repository at this point in the history
* elf: fail validation if writable sections are present
* Featurize .bss reject

Co-authored-by: Jack May <[email protected]>
  • Loading branch information
alessandrod and jackcmay authored Dec 14, 2021
1 parent 1992512 commit c56ccaa
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 15 deletions.
76 changes: 61 additions & 15 deletions src/elf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ pub enum ElfError {
/// Read-write data not supported
#[error("Found .bss section in ELF, read-write data not supported")]
BssNotSupported,
/// Read-write data not supported
#[error("Found writable section ({0}) in ELF, read-write data not supported")]
WritableSectionNotSupported(String),
/// Relocation failed, no loadable section contains virtual address
#[error("Relocation failed, no loadable section contains virtual address {0:#x}")]
AddressOutsideLoadableSection(u64),
Expand Down Expand Up @@ -378,7 +381,7 @@ impl<E: UserDefinedError, I: InstructionMeter> Executable<E, I> {
let elf = Elf::parse(bytes)?;
let mut elf_bytes = AlignedMemory::new_with_data(bytes, ebpf::HOST_ALIGN);

Self::validate(&elf, elf_bytes.as_slice())?;
Self::validate(&config, &elf, elf_bytes.as_slice())?;

// calculate the text section info
let text_section = Self::get_section(&elf, ".text")?;
Expand Down Expand Up @@ -536,7 +539,7 @@ impl<E: UserDefinedError, I: InstructionMeter> Executable<E, I> {
}

/// Validates the ELF
pub fn validate(elf: &Elf, elf_bytes: &[u8]) -> Result<(), ElfError> {
pub fn validate(config: &Config, elf: &Elf, elf_bytes: &[u8]) -> Result<(), ElfError> {
if elf.header.e_ident[EI_CLASS] != ELFCLASS64 {
return Err(ElfError::WrongClass);
}
Expand Down Expand Up @@ -569,8 +572,14 @@ impl<E: UserDefinedError, I: InstructionMeter> Executable<E, I> {
}

for section_header in elf.section_headers.iter() {
if let Some(this_name) = elf.shdr_strtab.get_at(section_header.sh_name) {
if this_name.starts_with(".bss") {
if let Some(name) = elf.shdr_strtab.get_at(section_header.sh_name) {
if config.reject_all_writable_sections
&& (name.starts_with(".bss")
|| (section_header.is_writable()
&& (name.starts_with(".data") && !name.starts_with(".data.rel"))))
{
return Err(ElfError::WritableSectionNotSupported(name.to_owned()));
} else if name == ".bss" {
return Err(ElfError::BssNotSupported);
}
}
Expand Down Expand Up @@ -862,28 +871,30 @@ mod test {
.expect("failed to read elf file");
let mut parsed_elf = Elf::parse(&bytes).unwrap();
let elf_bytes = bytes.to_vec();
let config = Config::default();

ElfExecutable::validate(&parsed_elf, &elf_bytes).expect("validation failed");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes).expect("validation failed");
parsed_elf.header.e_ident[EI_CLASS] = ELFCLASS32;
ElfExecutable::validate(&parsed_elf, &elf_bytes).expect_err("allowed bad class");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes).expect_err("allowed bad class");
parsed_elf.header.e_ident[EI_CLASS] = ELFCLASS64;
ElfExecutable::validate(&parsed_elf, &elf_bytes).expect("validation failed");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes).expect("validation failed");
parsed_elf.header.e_ident[EI_DATA] = ELFDATA2MSB;
ElfExecutable::validate(&parsed_elf, &elf_bytes).expect_err("allowed big endian");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes).expect_err("allowed big endian");
parsed_elf.header.e_ident[EI_DATA] = ELFDATA2LSB;
ElfExecutable::validate(&parsed_elf, &elf_bytes).expect("validation failed");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes).expect("validation failed");
parsed_elf.header.e_ident[EI_OSABI] = 1;
ElfExecutable::validate(&parsed_elf, &elf_bytes).expect_err("allowed wrong abi");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes).expect_err("allowed wrong abi");
parsed_elf.header.e_ident[EI_OSABI] = ELFOSABI_NONE;
ElfExecutable::validate(&parsed_elf, &elf_bytes).expect("validation failed");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes).expect("validation failed");
parsed_elf.header.e_machine = EM_QDSP6;
ElfExecutable::validate(&parsed_elf, &elf_bytes).expect_err("allowed wrong machine");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes)
.expect_err("allowed wrong machine");
parsed_elf.header.e_machine = EM_BPF;
ElfExecutable::validate(&parsed_elf, &elf_bytes).expect("validation failed");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes).expect("validation failed");
parsed_elf.header.e_type = ET_REL;
ElfExecutable::validate(&parsed_elf, &elf_bytes).expect_err("allowed wrong type");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes).expect_err("allowed wrong type");
parsed_elf.header.e_type = ET_DYN;
ElfExecutable::validate(&parsed_elf, &elf_bytes).expect("validation failed");
ElfExecutable::validate(&config, &parsed_elf, &elf_bytes).expect("validation failed");
}

#[test]
Expand Down Expand Up @@ -1180,4 +1191,39 @@ mod test {
ElfExecutable::load(Config::default(), &elf_bytes, syscall_registry())
.expect("validation failed");
}

#[test]
#[should_panic(expected = r#"validation failed: WritableSectionNotSupported(".data")"#)]
fn test_writable_data_section() {
let elf_bytes =
std::fs::read("tests/elfs/writable_data_section.so").expect("failed to read elf file");

assert!(ElfExecutable::load(Config::default(), &elf_bytes, syscall_registry()).is_ok());

ElfExecutable::load(
Config {
reject_all_writable_sections: true,
..Config::default()
},
&elf_bytes,
syscall_registry(),
)
.expect("validation failed");
}

#[test]
#[should_panic(expected = r#"validation failed: WritableSectionNotSupported(".bss")"#)]
fn test_bss_section() {
let elf_bytes =
std::fs::read("tests/elfs/bss_section.so").expect("failed to read elf file");
ElfExecutable::load(
Config {
reject_all_writable_sections: true,
..Config::default()
},
&elf_bytes,
syscall_registry(),
)
.expect("validation failed");
}
}
3 changes: 3 additions & 0 deletions src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ pub struct Config {
pub reject_unresolved_syscalls: bool,
/// Reject ELF files containing section headers where sh_addr != sh_offset
pub reject_section_virtual_address_file_offset_mismatch: bool,
/// Reject ELF files containing writable data sections
pub reject_all_writable_sections: bool,
/// Ratio of random no-ops per instruction in JIT (0.0 = OFF)
pub noop_instruction_ratio: f64,
/// Enable disinfection of immediate values and offsets provided by the user in JIT
Expand All @@ -217,6 +219,7 @@ impl Default for Config {
enable_symbol_and_section_labels: false,
reject_unresolved_syscalls: false,
reject_section_virtual_address_file_offset_mismatch: false,
reject_all_writable_sections: false,
noop_instruction_ratio: 1.0 / 256.0,
sanitize_user_provided_values: true,
encrypt_environment_registers: true,
Expand Down
9 changes: 9 additions & 0 deletions tests/elfs/bss_section.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
typedef unsigned long int uint64_t;
typedef unsigned char uint8_t;

int val = 0;

extern uint64_t entrypoint(const uint8_t *input) {
val = 43;
return 0;
}
Binary file added tests/elfs/bss_section.so
Binary file not shown.
8 changes: 8 additions & 0 deletions tests/elfs/elfs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,11 @@ rm scratch_registers.o
"$LLVM_DIR"clang -Werror -target bpf -O2 -fno-builtin -fPIC -o pass_stack_reference.o -c pass_stack_reference.c
"$LLVM_DIR"ld.lld -z notext -shared --Bdynamic -entry entrypoint -o pass_stack_reference.so pass_stack_reference.o
rm pass_stack_reference.o

"$LLVM_DIR"clang -Werror -target bpf -O2 -fno-builtin -fPIC -o writable_data_section.o -c writable_data_section.c
"$LLVM_DIR"ld.lld -z notext -shared --Bdynamic -entry entrypoint --script elf.ld -o writable_data_section.so writable_data_section.o
rm writable_data_section.o

"$LLVM_DIR"clang -Werror -target bpf -O2 -fno-builtin -fPIC -o bss_section.o -c bss_section.c
"$LLVM_DIR"ld.lld -z notext -shared --Bdynamic -entry entrypoint -o bss_section.so bss_section.o
rm bss_section.o
9 changes: 9 additions & 0 deletions tests/elfs/writable_data_section.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
typedef unsigned long int uint64_t;
typedef unsigned char uint8_t;

int val = 42;

extern uint64_t entrypoint(const uint8_t *input) {
val = 43;
return 0;
}
Binary file added tests/elfs/writable_data_section.so
Binary file not shown.

0 comments on commit c56ccaa

Please sign in to comment.