From a1817b44d6865e4e696f2d2d24d1b6d4a9e180eb Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Sun, 6 Nov 2022 19:19:24 +0100 Subject: [PATCH 1/8] whoops, don't discard runtime errors --- boa_engine/src/context/mod.rs | 17 +++++++++++++++++ boa_engine/src/error.rs | 23 +++++++++++++++++++++++ boa_engine/src/vm/mod.rs | 9 ++++++++- fuzz/Cargo.toml | 6 ++++++ fuzz/README.md | 13 +++++++++++++ fuzz/fuzz_targets/common.rs | 24 +++++++++++++++++++++++- fuzz/fuzz_targets/vm-implied.rs | 19 +++++++++++++++++++ 7 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 fuzz/fuzz_targets/vm-implied.rs diff --git a/boa_engine/src/context/mod.rs b/boa_engine/src/context/mod.rs index 47cc742b3d1..7597ee81d6a 100644 --- a/boa_engine/src/context/mod.rs +++ b/boa_engine/src/context/mod.rs @@ -97,6 +97,10 @@ pub struct Context { #[cfg(feature = "intl")] icu: icu::Icu, + /// Number of instructions remaining before a forced exit + #[cfg(feature = "fuzz")] + pub(crate) insns_remaining: usize, + pub(crate) vm: Vm, pub(crate) promise_job_queue: VecDeque, @@ -593,6 +597,8 @@ pub struct ContextBuilder { interner: Option, #[cfg(feature = "intl")] icu: Option, + #[cfg(feature = "fuzz")] + insns_remaining: usize, } impl ContextBuilder { @@ -615,6 +621,15 @@ impl ContextBuilder { Ok(self) } + /// Specifies the number of instructions remaining to the [`Context`]. + /// + /// This function is only available if the `fuzz` feature is enabled. + #[cfg(feature = "fuzz")] + pub fn insns_remaining(mut self, insns_remaining: usize) -> Self { + self.insns_remaining = insns_remaining; + self + } + /// Creates a new [`ContextBuilder`] with a default empty [`Interner`] /// and a default [`BoaProvider`] if the `intl` feature is enabled. pub fn new() -> Self { @@ -643,6 +658,8 @@ impl ContextBuilder { icu::Icu::new(Box::new(icu_testdata::get_provider())) .expect("Failed to initialize default icu data.") }), + #[cfg(feature = "fuzz")] + insns_remaining: self.insns_remaining, promise_job_queue: VecDeque::new(), }; diff --git a/boa_engine/src/error.rs b/boa_engine/src/error.rs index 00e124da31f..115a52f6672 100644 --- a/boa_engine/src/error.rs +++ b/boa_engine/src/error.rs @@ -503,6 +503,17 @@ impl JsNativeError { Self::new(JsNativeErrorKind::Uri, Box::default(), None) } + /// Creates a new `JsNativeError` that indicates that the context hit its execution limit. This + /// is only used in a fuzzing context. + #[cfg(feature = "fuzz")] + pub fn no_instructions_remain() -> Self { + Self::new( + JsNativeErrorKind::NoInstructionsRemain, + Box::default(), + None, + ) + } + /// Sets the message of this error. /// /// # Examples @@ -619,6 +630,12 @@ impl JsNativeError { } JsNativeErrorKind::Type => (constructors.type_error().prototype(), ErrorKind::Type), JsNativeErrorKind::Uri => (constructors.uri_error().prototype(), ErrorKind::Uri), + #[cfg(feature = "fuzz")] + JsNativeErrorKind::NoInstructionsRemain => { + // we can propagate out from try/catch since the catch block will also perform some + // operation + (constructors.error().prototype(), ErrorKind::Error) + } }; let o = JsObject::from_proto_and_data(prototype, ObjectData::error(tag)); @@ -747,6 +764,10 @@ pub enum JsNativeErrorKind { /// [e_uri]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURI /// [d_uri]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURI Uri, + /// Error thrown when no instructions remain. Only used in a fuzzing context; not a valid JS + /// error variant. + #[cfg(feature = "fuzz")] + NoInstructionsRemain, } impl std::fmt::Display for JsNativeErrorKind { @@ -760,6 +781,8 @@ impl std::fmt::Display for JsNativeErrorKind { JsNativeErrorKind::Syntax => "SyntaxError", JsNativeErrorKind::Type => "TypeError", JsNativeErrorKind::Uri => "UriError", + #[cfg(feature = "fuzz")] + JsNativeErrorKind::NoInstructionsRemain => "NoInstructionsRemain", } .fmt(f) } diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index 49ba10667e1..de7350a3bba 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -5,7 +5,7 @@ use crate::{ builtins::async_generator::{AsyncGenerator, AsyncGeneratorState}, vm::{call_frame::CatchAddresses, code_block::Readable}, - Context, JsResult, JsValue, + Context, JsError, JsNativeError, JsResult, JsValue, }; use boa_interner::ToInternedString; use boa_profiler::Profiler; @@ -112,6 +112,13 @@ pub(crate) enum ReturnType { impl Context { fn execute_instruction(&mut self) -> JsResult { + #[cfg(feature = "fuzz")] + if self.insns_remaining == 0 { + return Err(JsError::from_native(JsNativeError::no_instructions_remain())); + } else { + self.insns_remaining -= 1; + } + let opcode: Opcode = { let _timer = Profiler::global().start_event("Opcode retrieval", "vm"); let opcode = self.vm.frame().code.code[self.vm.frame().pc] diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index cc86d751da9..2bb90fee011 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -27,3 +27,9 @@ name = "parser-idempotency" path = "fuzz_targets/parser-idempotency.rs" test = false doc = false + +[[bin]] +name = "vm-implied" +path = "fuzz_targets/vm-implied.rs" +test = false +doc = false diff --git a/fuzz/README.md b/fuzz/README.md index 097e46ccb97..a3a1e77f183 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -35,3 +35,16 @@ following: information, as the inputs parsed between the two should be the same. In this way, this fuzzer can identify correctness issues present in the parser. + +## VM Fuzzer + +The VM fuzzer, located in [vm-implied.rs](fuzz_targets/vm-implied.rs), identifies crash cases in the VM. It does so by +generating an arbitrary AST, converting it to source code (to remove invalid inputs), then executing that source code. +Because we are not comparing against any invariants other than "does it crash", this fuzzer will only discover faults +which cause the VM to terminate unexpectedly, e.g. as a result of a panic. It will not discover logic errors present in +the VM. + +To ensure that the VM does not attempt to execute an infinite loop, Boa is restricted to a finite number of instructions +before the VM is terminated. If a program takes more than a second or so to execute, it likely indicates an issue in the +VM (as we expect the fuzzer to execute only a certain amount of instructions, which should take significantly less +time). diff --git a/fuzz/fuzz_targets/common.rs b/fuzz/fuzz_targets/common.rs index 161a6145c00..8d4c50b5425 100644 --- a/fuzz/fuzz_targets/common.rs +++ b/fuzz/fuzz_targets/common.rs @@ -2,7 +2,7 @@ use boa_ast::{ visitor::{VisitWith, VisitorMut}, Expression, StatementList, }; -use boa_interner::{Interner, Sym}; +use boa_interner::{Interner, Sym, ToInternedString}; use libfuzzer_sys::arbitrary; use libfuzzer_sys::arbitrary::{Arbitrary, Unstructured}; use std::fmt::{Debug, Formatter}; @@ -72,3 +72,25 @@ impl Debug for FuzzData { .finish_non_exhaustive() } } + +pub struct FuzzSource { + pub interner: Interner, + pub source: String, +} + +impl<'a> Arbitrary<'a> for FuzzSource { + fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { + let data = FuzzData::arbitrary(u)?; + let source = data.ast.to_interned_string(&data.interner); + Ok(Self { + interner: data.interner, + source, + }) + } +} + +impl Debug for FuzzSource { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.write_fmt(format_args!("Fuzzed source:\n{}", self.source)) + } +} diff --git a/fuzz/fuzz_targets/vm-implied.rs b/fuzz/fuzz_targets/vm-implied.rs new file mode 100644 index 00000000000..6086bb014cb --- /dev/null +++ b/fuzz/fuzz_targets/vm-implied.rs @@ -0,0 +1,19 @@ +#![no_main] + +mod common; + +use crate::common::FuzzSource; +use boa_engine::{Context, JsResult, JsValue}; +use libfuzzer_sys::fuzz_target; + +fn do_fuzz(original: FuzzSource) -> JsResult { + let mut ctx = Context::builder() + .interner(original.interner) + .insns_remaining(1 << 16) + .build(); + ctx.eval(&original.source) +} + +fuzz_target!(|original: FuzzSource| { + let _ = do_fuzz(original); +}); From 19a3d008bed2465e79c56899b174f74c90e2d2cb Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Sun, 6 Nov 2022 21:22:25 +0100 Subject: [PATCH 2/8] make the NoInstructionsRemaining error variant uncatchable --- boa_engine/src/error.rs | 6 +++--- boa_engine/src/vm/mod.rs | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/boa_engine/src/error.rs b/boa_engine/src/error.rs index 115a52f6672..c76bee7f336 100644 --- a/boa_engine/src/error.rs +++ b/boa_engine/src/error.rs @@ -632,9 +632,9 @@ impl JsNativeError { JsNativeErrorKind::Uri => (constructors.uri_error().prototype(), ErrorKind::Uri), #[cfg(feature = "fuzz")] JsNativeErrorKind::NoInstructionsRemain => { - // we can propagate out from try/catch since the catch block will also perform some - // operation - (constructors.error().prototype(), ErrorKind::Error) + unreachable!( + "The NoInstructionsRemain native error cannot be converted to an opaque type." + ) } }; diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index de7350a3bba..c48bc28d0a2 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -112,13 +112,6 @@ pub(crate) enum ReturnType { impl Context { fn execute_instruction(&mut self) -> JsResult { - #[cfg(feature = "fuzz")] - if self.insns_remaining == 0 { - return Err(JsError::from_native(JsNativeError::no_instructions_remain())); - } else { - self.insns_remaining -= 1; - } - let opcode: Opcode = { let _timer = Profiler::global().start_event("Opcode retrieval", "vm"); let opcode = self.vm.frame().code.code[self.vm.frame().pc] @@ -186,6 +179,13 @@ impl Context { }); while self.vm.frame().pc < self.vm.frame().code.code.len() { + #[cfg(feature = "fuzz")] + if self.insns_remaining == 0 { + return Err(JsError::from_native(JsNativeError::no_instructions_remain())); + } else { + self.insns_remaining -= 1; + } + let result = if self.vm.trace { let mut pc = self.vm.frame().pc; let opcode: Opcode = self From 5c808b10b436d68ea015d91e3b88d29275f9dd04 Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Mon, 7 Nov 2022 17:02:14 +0100 Subject: [PATCH 3/8] remove unnecessary use statements --- boa_engine/src/vm/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index c48bc28d0a2..667e5887b30 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -5,7 +5,7 @@ use crate::{ builtins::async_generator::{AsyncGenerator, AsyncGeneratorState}, vm::{call_frame::CatchAddresses, code_block::Readable}, - Context, JsError, JsNativeError, JsResult, JsValue, + Context, JsResult, JsValue, }; use boa_interner::ToInternedString; use boa_profiler::Profiler; From 817cea29c5e9dd0240dd12b25d1ae6825c83af82 Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Mon, 7 Nov 2022 17:10:54 +0100 Subject: [PATCH 4/8] add bytecompiler fuzzer --- boa_engine/src/vm/mod.rs | 2 ++ fuzz/Cargo.toml | 6 ++++++ fuzz/fuzz_targets/bytecompiler-implied.rs | 25 +++++++++++++++++++++++ 3 files changed, 33 insertions(+) create mode 100644 fuzz/fuzz_targets/bytecompiler-implied.rs diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index 667e5887b30..ec4a3fbe3c2 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -7,6 +7,8 @@ use crate::{ vm::{call_frame::CatchAddresses, code_block::Readable}, Context, JsResult, JsValue, }; +#[cfg(feature = "fuzz")] +use crate::{JsError, JsNativeError}; use boa_interner::ToInternedString; use boa_profiler::Profiler; use std::{convert::TryInto, mem::size_of, time::Instant}; diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 2bb90fee011..bdd84c79cf4 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -33,3 +33,9 @@ name = "vm-implied" path = "fuzz_targets/vm-implied.rs" test = false doc = false + +[[bin]] +name = "bytecompiler-implied" +path = "fuzz_targets/bytecompiler-implied.rs" +test = false +doc = false diff --git a/fuzz/fuzz_targets/bytecompiler-implied.rs b/fuzz/fuzz_targets/bytecompiler-implied.rs new file mode 100644 index 00000000000..21d72f83a99 --- /dev/null +++ b/fuzz/fuzz_targets/bytecompiler-implied.rs @@ -0,0 +1,25 @@ +#![no_main] + +mod common; + +use crate::common::FuzzSource; +use boa_engine::Context; +use boa_parser::Parser; +use libfuzzer_sys::{fuzz_target, Corpus}; +use std::io::Cursor; + +fn do_fuzz(original: FuzzSource) -> Corpus { + let mut ctx = Context::builder() + .interner(original.interner) + .insns_remaining(0) + .build(); + let mut parser = Parser::new(Cursor::new(&original.source)); + if let Ok(parsed) = parser.parse_all(ctx.interner_mut()) { + let _ = ctx.compile(&parsed); + Corpus::Keep + } else { + Corpus::Reject + } +} + +fuzz_target!(|original: FuzzSource| -> Corpus { do_fuzz(original) }); From e2059516c1b5e47d4c56182b6602f8210087d18e Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Mon, 7 Nov 2022 17:12:46 +0100 Subject: [PATCH 5/8] Add docs for bytecompiler fuzzer --- fuzz/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fuzz/README.md b/fuzz/README.md index a3a1e77f183..9bb314bfd6b 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -36,6 +36,12 @@ following: In this way, this fuzzer can identify correctness issues present in the parser. +## Bytecompiler Fuzzer + +The bytecompiler fuzzer, located in [bytecompiler-implied.rs](fuzz_targets/bytecompiler-implied.rs), identifies cases +which cause an assertion failure in the bytecompiler. These crashes can cause denial of service issues and may block the +discovery of crash cases in the VM fuzzer. + ## VM Fuzzer The VM fuzzer, located in [vm-implied.rs](fuzz_targets/vm-implied.rs), identifies crash cases in the VM. It does so by From 3e75e483d91fdcfcc3d37ec311f4cf0146d3029b Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Mon, 14 Nov 2022 14:27:03 +0100 Subject: [PATCH 6/8] refactor: insns => instructions --- boa_engine/src/context/mod.rs | 10 +++++----- boa_engine/src/vm/mod.rs | 4 ++-- fuzz/fuzz_targets/bytecompiler-implied.rs | 2 +- fuzz/fuzz_targets/vm-implied.rs | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/boa_engine/src/context/mod.rs b/boa_engine/src/context/mod.rs index 7597ee81d6a..cc9691d4f58 100644 --- a/boa_engine/src/context/mod.rs +++ b/boa_engine/src/context/mod.rs @@ -99,7 +99,7 @@ pub struct Context { /// Number of instructions remaining before a forced exit #[cfg(feature = "fuzz")] - pub(crate) insns_remaining: usize, + pub(crate) instructions_remaining: usize, pub(crate) vm: Vm, @@ -598,7 +598,7 @@ pub struct ContextBuilder { #[cfg(feature = "intl")] icu: Option, #[cfg(feature = "fuzz")] - insns_remaining: usize, + instructions_remaining: usize, } impl ContextBuilder { @@ -625,8 +625,8 @@ impl ContextBuilder { /// /// This function is only available if the `fuzz` feature is enabled. #[cfg(feature = "fuzz")] - pub fn insns_remaining(mut self, insns_remaining: usize) -> Self { - self.insns_remaining = insns_remaining; + pub fn instructions_remaining(mut self, instructions_remaining: usize) -> Self { + self.instructions_remaining = instructions_remaining; self } @@ -659,7 +659,7 @@ impl ContextBuilder { .expect("Failed to initialize default icu data.") }), #[cfg(feature = "fuzz")] - insns_remaining: self.insns_remaining, + instructions_remaining: self.instructions_remaining, promise_job_queue: VecDeque::new(), }; diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index ec4a3fbe3c2..7f75db94aa2 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -182,10 +182,10 @@ impl Context { while self.vm.frame().pc < self.vm.frame().code.code.len() { #[cfg(feature = "fuzz")] - if self.insns_remaining == 0 { + if self.instructions_remaining == 0 { return Err(JsError::from_native(JsNativeError::no_instructions_remain())); } else { - self.insns_remaining -= 1; + self.instructions_remaining -= 1; } let result = if self.vm.trace { diff --git a/fuzz/fuzz_targets/bytecompiler-implied.rs b/fuzz/fuzz_targets/bytecompiler-implied.rs index 21d72f83a99..dd91bbc32a3 100644 --- a/fuzz/fuzz_targets/bytecompiler-implied.rs +++ b/fuzz/fuzz_targets/bytecompiler-implied.rs @@ -11,7 +11,7 @@ use std::io::Cursor; fn do_fuzz(original: FuzzSource) -> Corpus { let mut ctx = Context::builder() .interner(original.interner) - .insns_remaining(0) + .instructions_remaining(0) .build(); let mut parser = Parser::new(Cursor::new(&original.source)); if let Ok(parsed) = parser.parse_all(ctx.interner_mut()) { diff --git a/fuzz/fuzz_targets/vm-implied.rs b/fuzz/fuzz_targets/vm-implied.rs index 6086bb014cb..77d4c14b3b9 100644 --- a/fuzz/fuzz_targets/vm-implied.rs +++ b/fuzz/fuzz_targets/vm-implied.rs @@ -9,7 +9,7 @@ use libfuzzer_sys::fuzz_target; fn do_fuzz(original: FuzzSource) -> JsResult { let mut ctx = Context::builder() .interner(original.interner) - .insns_remaining(1 << 16) + .instructions_remaining(1 << 16) .build(); ctx.eval(&original.source) } From 49da04cf59e609a803297018103112293977cb96 Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Tue, 15 Nov 2022 11:11:52 +0100 Subject: [PATCH 7/8] update cargo lock --- fuzz/Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index beb643d96dc..7de351e39ac 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -167,9 +167,9 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "chrono" -version = "0.4.22" +version = "0.4.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bfd4d1b31faaa3a89d7934dbded3111da0d2ef28e3ebccdb4f0179f5929d1ef1" +checksum = "16b0a3d9ed01224b22057780a37bb8c5dbfe1be8ba48678e7bf57ec4b385411f" dependencies = [ "iana-time-zone", "js-sys", From 3c718d09338cdc5bbc9631786229473ee2c87491 Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Tue, 15 Nov 2022 13:34:06 +0100 Subject: [PATCH 8/8] update cargo lockfile --- fuzz/Cargo.lock | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 7de351e39ac..ab15b48ab56 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -59,7 +59,6 @@ dependencies = [ "chrono", "dyn-clone", "fast-float", - "gc", "indexmap", "num-bigint", "num-integer", @@ -93,7 +92,8 @@ dependencies = [ name = "boa_gc" version = "0.16.0" dependencies = [ - "gc", + "boa_macros", + "boa_profiler", ] [[package]] @@ -113,8 +113,10 @@ dependencies = [ name = "boa_macros" version = "0.16.0" dependencies = [ + "proc-macro2", "quote", "syn", + "synstructure", ] [[package]] @@ -263,27 +265,6 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "95765f67b4b18863968b4a1bd5bb576f732b29a4a28c7cd84c09fa3e2875f33c" -[[package]] -name = "gc" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3edaac0f5832202ebc99520cb77c932248010c4645d20be1dc62d6579f5b3752" -dependencies = [ - "gc_derive", -] - -[[package]] -name = "gc_derive" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "60df8444f094ff7885631d80e78eb7d88c3c2361a98daaabb06256e4500db941" -dependencies = [ - "proc-macro2", - "quote", - "syn", - "synstructure", -] - [[package]] name = "getrandom" version = "0.2.8"