Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get proper name for over-8-character sections #100

Merged
merged 5 commits into from
Sep 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/pe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ mod utils;
use error;
use container;

/// Size of a single symbol in the COFF Symbol Table.
const COFF_SYMBOL_SIZE: u32 = 18;

#[derive(Debug)]
/// An analyzed PE32/PE32+ binary
pub struct PE<'a> {
Expand Down Expand Up @@ -59,8 +62,10 @@ impl<'a> PE<'a> {
let offset = &mut (header.dos_header.pe_pointer as usize + header::SIZEOF_COFF_HEADER + header.coff_header.size_of_optional_header as usize);
let nsections = header.coff_header.number_of_sections as usize;
let mut sections = Vec::with_capacity(nsections);
// Note that if we are handling a BigCoff, the size of the symbol will be different!
let string_table_offset = header.coff_header.pointer_to_symbol_table + header.coff_header.number_of_symbol_table * COFF_SYMBOL_SIZE;
for i in 0..nsections {
let section = section_table::SectionTable::parse(bytes, offset)?;
let section = section_table::SectionTable::parse(bytes, offset, string_table_offset as usize)?;
debug!("({}) {:#?}", i, section);
sections.push(section);
}
Expand Down
54 changes: 50 additions & 4 deletions src/pe/section_table.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use scroll::{self, Pread};
use error;
use error::{self, Error};

#[repr(C)]
#[derive(Debug, PartialEq, Copy, Clone, Default)]
#[derive(Debug, PartialEq, Clone, Default)]
pub struct SectionTable {
pub name: [u8; 8],
pub real_name: Option<String>,
pub virtual_size: u32,
pub virtual_address: u32,
pub size_of_raw_data: u32,
Expand All @@ -18,13 +19,40 @@ pub struct SectionTable {

pub const SIZEOF_SECTION_TABLE: usize = 8 * 5;

// Based on https://github.com/llvm-mirror/llvm/blob/af7b1832a03ab6486c42a40d21695b2c03b2d8a3/lib/Object/COFFObjectFile.cpp#L70
// Decodes a string table entry in base 64 (//AAAAAA). Expects string without
// prefixed slashes.
fn base64_decode_string_entry(s: &str) -> Result<usize, ()> {
assert!(s.len() <= 6, "String too long, possible overflow.");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should assert unless it's a fundamental programmer error somehow. Return an Err is better

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the same, but looking into it I don't think it's possible for this to assert because the section name is at most 8 bytes, and 2 of those are slashes.

Copy link
Contributor Author

@roblabla roblabla Aug 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not return an error. If this assert is triggered, then something went terribly wrong, and we absolutely should abort.


let mut val = 0;
for c in s.bytes() {
let v = if b'A' <= c && c <= b'Z' {
c - b'A' + 00 // 00..=25
} else if b'a' <= c && c <= b'z' {
c - b'a' + 26 // 26..=51
} else if b'0' <= c && c <= b'9' {
c - b'0' + 52 // 52..=61
} else if c == b'+' {
62 // 62
} else if c == b'/' {
63 // 63
} else {
return Err(())
};
val = val * 64 + v as usize;
}
Ok(val)
}

impl SectionTable {
pub fn parse(bytes: &[u8], offset: &mut usize) -> error::Result<Self> {
pub fn parse(bytes: &[u8], offset: &mut usize, string_table_offset: usize) -> error::Result<Self> {
let mut table = SectionTable::default();
let mut name = [0u8; 8];
for i in 0..8 {
name[i] = bytes.gread_with(offset, scroll::LE)?;
}

table.name = name;
table.virtual_size = bytes.gread_with(offset, scroll::LE)?;
table.virtual_address = bytes.gread_with(offset, scroll::LE)?;
Expand All @@ -35,10 +63,28 @@ impl SectionTable {
table.number_of_relocations = bytes.gread_with(offset, scroll::LE)?;
table.number_of_linenumbers = bytes.gread_with(offset, scroll::LE)?;
table.characteristics = bytes.gread_with(offset, scroll::LE)?;

// Based on https://github.com/llvm-mirror/llvm/blob/af7b1832a03ab6486c42a40d21695b2c03b2d8a3/lib/Object/COFFObjectFile.cpp#L1054
if name[0] == b'/' {
let idx: usize = if name[1] == b'/' {
let b64idx = name.pread::<&str>(2)?;
base64_decode_string_entry(b64idx).map_err(|_|
Error::Malformed(format!("Invalid indirect section name //{}: base64 decoding failed", b64idx)))?
} else {
let name = name.pread::<&str>(1)?;
name.parse().map_err(|err|
Error::Malformed(format!("Invalid indirect section name /{}: {}", name, err)))?
};
table.real_name = Some(bytes.pread::<&str>(string_table_offset + idx)?.to_string());
}
Ok(table)
}

pub fn name(&self) -> error::Result<&str> {
Ok(self.name.pread(0)?)
match self.real_name.as_ref() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this as_ref() here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I run into borrow errors otherwise. The second as_ref in the Some case isn't necessary though.

Some(s) => Ok(s),
None => Ok(self.name.pread(0)?)
}
}
}

Expand Down