From 7c9302f6a788e9676d4804e84e5d7af5e3696997 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 22 Mar 2021 12:19:28 -0400 Subject: [PATCH] Fix a number of bugs in zbase32 and add a fuzzer which caught them. --- fuzz/src/bin/gen_target.sh | 1 + fuzz/src/bin/zbase32_target.rs | 102 +++++++++++++++++++++++++++++++++ fuzz/src/lib.rs | 1 + fuzz/src/zbase32.rs | 33 +++++++++++ fuzz/targets.h | 1 + lightning/src/util/zbase32.rs | 47 ++++++++++----- 6 files changed, 172 insertions(+), 13 deletions(-) create mode 100644 fuzz/src/bin/zbase32_target.rs create mode 100644 fuzz/src/zbase32.rs diff --git a/fuzz/src/bin/gen_target.sh b/fuzz/src/bin/gen_target.sh index 8cb52f668..eb07df634 100755 --- a/fuzz/src/bin/gen_target.sh +++ b/fuzz/src/bin/gen_target.sh @@ -11,6 +11,7 @@ GEN_TEST chanmon_consistency GEN_TEST full_stack GEN_TEST peer_crypt GEN_TEST router +GEN_TEST zbase32 GEN_TEST msg_accept_channel msg_targets:: GEN_TEST msg_announcement_signatures msg_targets:: diff --git a/fuzz/src/bin/zbase32_target.rs b/fuzz/src/bin/zbase32_target.rs new file mode 100644 index 000000000..ae96ce409 --- /dev/null +++ b/fuzz/src/bin/zbase32_target.rs @@ -0,0 +1,102 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +// This file is auto-generated by gen_target.sh based on target_template.txt +// To modify it, modify target_template.txt and run gen_target.sh instead. + +#![cfg_attr(feature = "libfuzzer_fuzz", no_main)] + +extern crate lightning_fuzz; +use lightning_fuzz::zbase32::*; + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + zbase32_run(data.as_ptr(), data.len()); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + zbase32_run(data.as_ptr(), data.len()); + }); + } +} + +#[cfg(feature = "libfuzzer_fuzz")] +#[macro_use] extern crate libfuzzer_sys; +#[cfg(feature = "libfuzzer_fuzz")] +fuzz_target!(|data: &[u8]| { + zbase32_run(data.as_ptr(), data.len()); +}); + +#[cfg(feature = "stdin_fuzz")] +fn main() { + use std::io::Read; + + let mut data = Vec::with_capacity(8192); + std::io::stdin().read_to_end(&mut data).unwrap(); + zbase32_run(data.as_ptr(), data.len()); +} + +#[test] +fn run_test_cases() { + use std::fs; + use std::io::Read; + use lightning_fuzz::utils::test_logger::StringBuffer; + + use std::sync::{atomic, Arc}; + { + let data: Vec = vec![0]; + zbase32_run(data.as_ptr(), data.len()); + } + let mut threads = Vec::new(); + let threads_running = Arc::new(atomic::AtomicUsize::new(0)); + if let Ok(tests) = fs::read_dir("test_cases/zbase32") { + for test in tests { + let mut data: Vec = Vec::new(); + let path = test.unwrap().path(); + fs::File::open(&path).unwrap().read_to_end(&mut data).unwrap(); + threads_running.fetch_add(1, atomic::Ordering::AcqRel); + + let thread_count_ref = Arc::clone(&threads_running); + let main_thread_ref = std::thread::current(); + threads.push((path.file_name().unwrap().to_str().unwrap().to_string(), + std::thread::spawn(move || { + let string_logger = StringBuffer::new(); + + let panic_logger = string_logger.clone(); + let res = if ::std::panic::catch_unwind(move || { + zbase32_test(&data, panic_logger); + }).is_err() { + Some(string_logger.into_string()) + } else { None }; + thread_count_ref.fetch_sub(1, atomic::Ordering::AcqRel); + main_thread_ref.unpark(); + res + }) + )); + while threads_running.load(atomic::Ordering::Acquire) > 32 { + std::thread::park(); + } + } + } + for (test, thread) in threads.drain(..) { + if let Some(output) = thread.join().unwrap() { + println!("Output of {}:\n{}", test, output); + panic!(); + } + } +} diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index 5fe50520c..a0cc42b81 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -18,5 +18,6 @@ pub mod chanmon_consistency; pub mod full_stack; pub mod peer_crypt; pub mod router; +pub mod zbase32; pub mod msg_targets; diff --git a/fuzz/src/zbase32.rs b/fuzz/src/zbase32.rs new file mode 100644 index 000000000..305485b3e --- /dev/null +++ b/fuzz/src/zbase32.rs @@ -0,0 +1,33 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +use lightning::util::zbase32; + +use utils::test_logger; + +#[inline] +pub fn do_test(data: &[u8]) { + let res = zbase32::encode(data); + assert_eq!(&zbase32::decode(&res).unwrap()[..], data); + + if let Ok(s) = std::str::from_utf8(data) { + if let Ok(decoded) = zbase32::decode(s) { + assert_eq!(&zbase32::encode(&decoded), &s.to_ascii_lowercase()); + } + } +} + +pub fn zbase32_test(data: &[u8], _out: Out) { + do_test(data); +} + +#[no_mangle] +pub extern "C" fn zbase32_run(data: *const u8, datalen: usize) { + do_test(unsafe { std::slice::from_raw_parts(data, datalen) }); +} diff --git a/fuzz/targets.h b/fuzz/targets.h index 7ca3e93f3..5d45e3d02 100644 --- a/fuzz/targets.h +++ b/fuzz/targets.h @@ -4,6 +4,7 @@ void chanmon_consistency_run(const unsigned char* data, size_t data_len); void full_stack_run(const unsigned char* data, size_t data_len); void peer_crypt_run(const unsigned char* data, size_t data_len); void router_run(const unsigned char* data, size_t data_len); +void zbase32_run(const unsigned char* data, size_t data_len); void msg_accept_channel_run(const unsigned char* data, size_t data_len); void msg_announcement_signatures_run(const unsigned char* data, size_t data_len); void msg_channel_reestablish_run(const unsigned char* data, size_t data_len); diff --git a/lightning/src/util/zbase32.rs b/lightning/src/util/zbase32.rs index 19a10cca3..b9b204e5e 100644 --- a/lightning/src/util/zbase32.rs +++ b/lightning/src/util/zbase32.rs @@ -25,8 +25,9 @@ SOFTWARE. const ALPHABET: &'static [u8] = b"ybndrfg8ejkmcpqxot1uwisza345h769"; +/// Encodes some bytes as a zbase32 string pub fn encode(data: &[u8]) -> String { - let mut ret = Vec::with_capacity((data.len() + 3) / 4 * 5); + let mut ret = Vec::with_capacity((data.len() + 4) / 5 * 8); for chunk in data.chunks(5) { let buf = { @@ -47,11 +48,11 @@ pub fn encode(data: &[u8]) -> String { ret.push(ALPHABET[(buf[4] & 0x1F) as usize]); } - if data.len() % 5 != 0 { - let len = ret.len(); - let num_extra = 8 - (data.len() % 5 * 8 + 4) / 5; - ret.truncate(len - num_extra); - } + ret.truncate((data.len() * 8 + 4) / 5); + + // Check that our capacity calculation doesn't under-shoot in fuzzing + #[cfg(fuzzing)] + assert_eq!(ret.capacity(), (data.len() + 4) / 5 * 8); String::from_utf8(ret).unwrap() } @@ -62,21 +63,29 @@ const INV_ALPHABET: [i8; 43] = [ 21, 9, 10, -1, 11, 2, 16, 13, 14, 4, 22, 17, 19, -1, 20, 15, 0, 23, ]; -pub fn decode(data: &str) -> Result, &'static str> { +/// Decodes a zbase32 string to the original bytes, failing if the string was not encoded by a +/// proper zbase32 encoder. +pub fn decode(data: &str) -> Result, ()> { if !data.is_ascii() { - return Err("Data is not zbase32 encoded"); + return Err(()); } let data = data.as_bytes(); let output_length = data.len() * 5 / 8; - let mut ret = Vec::with_capacity((output_length + 4) / 5 * 5); + if data.len() > (output_length * 8 + 4) / 5 { + // If the string has more charachters than are required to encode the number of bytes + // decodable, treat the string as invalid. + return Err(()); + } + + let mut ret = Vec::with_capacity((data.len() + 7) / 8 * 5); for chunk in data.chunks(8) { let buf = { let mut buf = [0u8; 8]; for (i, &c) in chunk.iter().enumerate() { match INV_ALPHABET.get(c.to_ascii_uppercase().wrapping_sub(b'0') as usize) { - Some(&-1) | None => return Err("Data is not zbase32 encoded"), + Some(&-1) | None => return Err(()), Some(&value) => buf[i] = value as u8, }; } @@ -88,10 +97,22 @@ pub fn decode(data: &str) -> Result, &'static str> { ret.push((buf[4] << 7) | (buf[5] << 2) | (buf[6] >> 3)); ret.push((buf[6] << 5) | buf[7]); } - ret.truncate(output_length); + for c in ret.drain(output_length..) { + if c != 0 { + // If the original string had any bits set at positions outside of the encoded data, + // treat the string as invalid. + return Err(()); + } + } + + // Check that our capacity calculation doesn't under-shoot in fuzzing + #[cfg(fuzzing)] + assert_eq!(ret.capacity(), (data.len() + 7) / 8 * 5); + Ok(ret) } +#[cfg(test)] mod tests { use super::*; @@ -125,7 +146,7 @@ mod tests { #[test] fn test_decode_wrong() { - let WRONG_DATA = &["00", "l1", "?", "="]; + const WRONG_DATA: &[&str] = &["00", "l1", "?", "="]; for &data in WRONG_DATA { match decode(data) { @@ -134,4 +155,4 @@ mod tests { } } } -} \ No newline at end of file +} -- 2.39.5