Skip to content

Commit

Permalink
Avoid leaking C FFI strings from invalid text chunks
Browse files Browse the repository at this point in the history 
Fixes #28
  • Loading branch information
@kornelski
kornelski committed Jul 18, 2018
1 parent 0a757b1 commit 1d052a2
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 53 deletions.
8 changes: 4 additions & 4 deletions src/ffi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,9 +486,9 @@ pub unsafe extern "C" fn lodepng_clear_text(info: &mut Info) {

#[no_mangle]
pub unsafe extern "C" fn lodepng_add_text(info: &mut Info, key: *const c_char, str: *const c_char) -> Error {
let k = CStr::from_ptr(key);
let s = CStr::from_ptr(str);
lode_error!(rustimpl::lodepng_add_text(info, k, s))
let k = lode_try!(CStr::from_ptr(key).to_str().map_err(|_| Error(89)));
let s = lode_try!(CStr::from_ptr(str).to_str().map_err(|_| Error(89)));
lode_error!(info.add_text(k, s))
}

#[no_mangle]
Expand All @@ -502,7 +502,7 @@ pub unsafe extern "C" fn lodepng_add_itext(info: &mut Info, key: *const c_char,
let l = CStr::from_ptr(langtag);
let t = CStr::from_ptr(transkey);
let s = CStr::from_ptr(str);
lode_error!(rustimpl::lodepng_add_itext(info, k, l, t, s))
lode_error!(info.push_itext(k.to_bytes(), l.to_bytes(), t.to_bytes(), s.to_bytes()))
}

#[no_mangle]
Expand Down
10 changes: 5 additions & 5 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ impl Info {

/// push back both texts at once
pub fn add_text(&mut self, key: &str, str: &str) -> Result<(), Error> {
self.push_text(string_copy_slice(key.as_bytes()), string_copy_slice(str.as_bytes()))
self.push_text(key.as_bytes(), str.as_bytes())
}

/// use this to clear the itexts again after you filled them in
Expand All @@ -346,10 +346,10 @@ impl Info {
/// push back the 4 texts of 1 chunk at once
pub fn add_itext(&mut self, key: &str, langtag: &str, transkey: &str, text: &str) -> Result<(), Error> {
self.push_itext(
string_copy_slice(key.as_bytes()),
string_copy_slice(langtag.as_bytes()),
string_copy_slice(transkey.as_bytes()),
string_copy_slice(text.as_bytes()),
key.as_bytes(),
langtag.as_bytes(),
transkey.as_bytes(),
text.as_bytes(),
)
}

Expand Down
69 changes: 25 additions & 44 deletions src/rustimpl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use super::*;
use ffi::ColorProfile;
use ffi::State;
use huffman::HuffmanTree;
use ChunkPosition;

pub use ucvec::*;
pub use rgb::RGBA8 as RGBA;
Expand Down Expand Up @@ -482,11 +483,7 @@ pub(crate) unsafe fn string_cleanup(out: &mut *mut c_char) {
*out = ptr::null_mut();
}

pub(crate) fn string_copy(inp: &CStr) -> *mut c_char {
string_copy_slice(inp.to_bytes())
}

pub(crate) fn string_copy_slice(inp: &[u8]) -> *mut c_char {
pub(crate) fn string_copy(inp: &[u8]) -> *mut c_char {
let insize = inp.len();
unsafe {
let out = lodepng_malloc(insize + 1) as *mut c_char;
Expand Down Expand Up @@ -529,43 +526,34 @@ pub(crate) fn text_copy(dest: &mut Info, source: &Info) -> Result<(), Error> {
dest.text_strings = ptr::null_mut();
dest.text_num = 0;
for (k, v) in source.text_keys_cstr() {
dest.push_text(string_copy(k), string_copy(v))?;
dest.push_text(k.to_bytes(), v.to_bytes())?;
}
Ok(())
}

unsafe fn realloc_push<T>(array: &mut *mut T, len: usize, new: T) -> Result<(), Error> {
*array = lodepng_realloc((*array) as *mut _, (len + 1) * mem::size_of::<T>()) as *mut _;
*array.offset(len as isize) = new;
unsafe fn realloc_push_string(array: &mut *mut *mut i8, len: usize, new: &[u8]) -> Result<(), Error> {
*array = lodepng_realloc((*array) as *mut _, (len + 1) * mem::size_of::<*mut i8>()) as *mut _;
*array.offset(len as isize) = string_copy(new);
Ok(())
}


use ChunkPosition;

impl Info {

pub(crate) fn push_itext(&mut self, key: *mut c_char, langtag: *mut c_char, transkey: *mut c_char, str: *mut c_char) -> Result<(), Error> {
assert!(!key.is_null());
assert!(!langtag.is_null());
assert!(!transkey.is_null());
assert!(!str.is_null());
pub(crate) fn push_itext(&mut self, key: &[u8], langtag: &[u8], transkey: &[u8], str: &[u8]) -> Result<(), Error> {
unsafe {
realloc_push(&mut self.itext_keys, self.itext_num, key)?;
realloc_push(&mut self.itext_langtags, self.itext_num, langtag)?;
realloc_push(&mut self.itext_transkeys, self.itext_num, transkey)?;
realloc_push(&mut self.itext_strings, self.itext_num, str)?;
realloc_push_string(&mut self.itext_keys, self.itext_num, key)?;
realloc_push_string(&mut self.itext_langtags, self.itext_num, langtag)?;
realloc_push_string(&mut self.itext_transkeys, self.itext_num, transkey)?;
realloc_push_string(&mut self.itext_strings, self.itext_num, str)?;
self.itext_num += 1;
}
Ok(())
}

pub(crate) fn push_text(&mut self, k: *mut c_char, v: *mut c_char) -> Result<(), Error> {
assert!(!k.is_null());
assert!(!v.is_null());
pub(crate) fn push_text(&mut self, k: &[u8], v: &[u8]) -> Result<(), Error> {
unsafe {
realloc_push(&mut self.text_keys, self.text_num, k)?;
realloc_push(&mut self.text_strings, self.text_num, v)?;
realloc_push_string(&mut self.text_keys, self.text_num, k)?;
realloc_push_string(&mut self.text_strings, self.text_num, v)?;
self.text_num += 1;
}
Ok(())
Expand Down Expand Up @@ -595,29 +583,21 @@ impl Info {
}
}

pub fn lodepng_add_text(info: &mut Info, key: &CStr, str: &CStr) -> Result<(), Error> {
info.push_text(string_copy(key), string_copy(str))
}

pub(crate) fn itext_copy(dest: &mut Info, source: &Info) -> Result<(), Error> {
dest.itext_keys = ptr::null_mut();
dest.itext_langtags = ptr::null_mut();
dest.itext_transkeys = ptr::null_mut();
dest.itext_strings = ptr::null_mut();
dest.itext_num = 0;
for (k,l,t,s) in source.itext_keys() {
dest.push_itext(string_copy_slice(k.as_bytes()),
string_copy_slice(l.as_bytes()),
string_copy_slice(t.as_bytes()),
string_copy_slice(s.as_bytes()))?;
dest.push_itext(k.as_bytes(),
l.as_bytes(),
t.as_bytes(),
s.as_bytes())?;
}
Ok(())
}

pub fn lodepng_add_itext(info: &mut Info, key: &CStr, langtag: &CStr, transkey: &CStr, str: &CStr) -> Result<(), Error> {
info.push_itext(string_copy(key), string_copy(langtag), string_copy(transkey), string_copy(str))
}

fn add_color_bits(out: &mut [u8], index: usize, bits: u32, mut inp: u32) {
let m = match bits {
1 => 7,
Expand Down Expand Up @@ -1206,7 +1186,7 @@ fn read_chunk_text(info: &mut Info, data: &[u8]) -> Result<(), Error> {
}
/*even though it's not allowed by the standard, no error is thrown if
there's no null termination char, if the text is empty*/
info.push_text(string_copy_slice(keyword), string_copy_slice(str))
info.push_text(keyword, str)
}

/*compressed text chunk (zTXt)*/
Expand All @@ -1232,7 +1212,7 @@ fn read_chunk_ztxt(info: &mut Info, zlibsettings: &DecompressSettings, data: &[u
}
let inl = &data[string2_begin..];
let decoded = zlib_decompress(inl, zlibsettings)?;
info.push_text(string_copy_slice(key), string_copy_slice(decoded.slice()))?;
info.push_text(key, decoded.slice())?;
Ok(())
}

Expand Down Expand Up @@ -1264,13 +1244,14 @@ fn read_chunk_itxt(info: &mut Info, zlibsettings: &DecompressSettings, data: &[u
let (langtag, data) = split_at_nul(&data[2..]);
let (transkey, data) = split_at_nul(data);

let decoded;
let rest = if compressed_flag != 0 {
let decoded = zlib_decompress(data, zlibsettings)?;
string_copy_slice(decoded.slice())
decoded = zlib_decompress(data, zlibsettings)?;
decoded.slice()
} else {
string_copy_slice(data)
data
};
info.push_itext(string_copy_slice(key), string_copy_slice(langtag), string_copy_slice(transkey), rest)?;
info.push_itext(key, langtag, transkey, rest)?;
Ok(())
}

Expand Down

0 comments on commit 1d052a2

Please sign in to comment.