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

Fix more fuzzing bugs. #786

Merged
merged 2 commits into from
Nov 27, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Bug fixes
* Fixed crash when parsing multi-segment interpolated string longer than maximum (found via fuzzing).
* Fixed crash when parsing unterminated comment (found via fuzzing).
* Fixed crash when parsing deeply-nested right-associated operators such as `**` (found via fuzzing).
* Fixed crash when parsing combo-chaining expressions such as `(a.b).c` (found via fuzzing).

Deprecated API's
----------------
Expand Down
4 changes: 2 additions & 2 deletions src/api/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::func::native::locked_write;
use crate::parser::{ParseSettingFlags, ParseState};
use crate::tokenizer::{lex_raw, Token};
use crate::types::StringsInterner;
use crate::{Engine, LexError, Map, OptimizationLevel, RhaiResultOf};
use crate::{Engine, LexError, Map, RhaiResultOf};
#[cfg(feature = "no_std")]
use std::prelude::v1::*;

Expand Down Expand Up @@ -134,7 +134,7 @@ impl Engine {
state,
|s| s.flags |= ParseSettingFlags::DISALLOW_UNQUOTED_MAP_PROPERTIES,
#[cfg(not(feature = "no_optimize"))]
OptimizationLevel::None,
crate::OptimizationLevel::None,
#[cfg(feature = "no_optimize")]
<_>::default(),
)?
Expand Down
24 changes: 15 additions & 9 deletions src/eval/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub struct FnResolutionCacheEntry {
#[derive(Debug, Clone, Default)]
pub struct FnResolutionCache {
/// Hash map containing cached functions.
pub map: StraightHashMap<Option<FnResolutionCacheEntry>>,
pub dict: StraightHashMap<Option<FnResolutionCacheEntry>>,
/// Bloom filter to avoid caching "one-hit wonders".
pub filter: BloomFilterU64,
}
Expand All @@ -34,7 +34,7 @@ impl FnResolutionCache {
#[inline(always)]
#[allow(dead_code)]
pub fn clear(&mut self) {
self.map.clear();
self.dict.clear();
self.filter.clear();
}
}
Expand All @@ -45,39 +45,45 @@ impl FnResolutionCache {
/// The following caches are contained inside this type:
/// * A stack of [function resolution caches][FnResolutionCache]
#[derive(Debug, Clone)]
pub struct Caches(StaticVec<FnResolutionCache>);
pub struct Caches {
fn_resolution: StaticVec<FnResolutionCache>,
}

impl Caches {
/// Create an empty [`Caches`].
#[inline(always)]
#[must_use]
pub const fn new() -> Self {
Self(StaticVec::new_const())
Self {
fn_resolution: StaticVec::new_const(),
}
}
/// Get the number of function resolution cache(s) in the stack.
#[inline(always)]
#[must_use]
pub fn fn_resolution_caches_len(&self) -> usize {
self.0.len()
self.fn_resolution.len()
}
/// Get a mutable reference to the current function resolution cache.
///
/// A new function resolution cache is pushed onto the stack if the stack is empty.
#[inline]
#[must_use]
pub fn fn_resolution_cache_mut(&mut self) -> &mut FnResolutionCache {
// Push a new function resolution cache if the stack is empty
if self.0.is_empty() {
if self.fn_resolution.is_empty() {
self.push_fn_resolution_cache();
}
self.0.last_mut().unwrap()
self.fn_resolution.last_mut().unwrap()
}
/// Push an empty function resolution cache onto the stack and make it current.
#[inline(always)]
pub fn push_fn_resolution_cache(&mut self) {
self.0.push(<_>::default());
self.fn_resolution.push(<_>::default());
}
/// Rewind the function resolution caches stack to a particular size.
#[inline(always)]
pub fn rewind_fn_resolution_caches(&mut self, len: usize) {
self.0.truncate(len);
self.fn_resolution.truncate(len);
}
}
4 changes: 2 additions & 2 deletions src/eval/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl Engine {
let this_ptr = this_ptr.as_deref_mut();

#[cfg(not(feature = "no_module"))]
let imports_len = global.num_imports();
let orig_imports_len = global.num_imports();

let result =
self.eval_stmt(global, caches, scope, this_ptr, stmt, restore_orig_state)?;
Expand All @@ -83,7 +83,7 @@ impl Engine {
// Without global functions, the extra modules never affect function resolution.
if global
.scan_imports_raw()
.skip(imports_len)
.skip(orig_imports_len)
.any(|(.., m)| m.contains_indexed_global_functions())
{
// Different scenarios where the cache must be cleared - notice that this is
Expand Down
6 changes: 4 additions & 2 deletions src/func/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl Engine {

let cache = caches.fn_resolution_cache_mut();

match cache.map.entry(hash) {
match cache.dict.entry(hash) {
Entry::Occupied(entry) => entry.into_mut().as_ref(),
Entry::Vacant(entry) => {
let num_args = args.as_deref().map_or(0, FnCallArgs::len);
Expand All @@ -190,22 +190,24 @@ impl Engine {
let mut bitmask = 1usize; // Bitmask of which parameter to replace with `Dynamic`

loop {
// First check scripted functions in the AST or embedded environments
#[cfg(not(feature = "no_function"))]
let func = _global
.lib
.iter()
.rev()
.chain(self.global_modules.iter())
.find_map(|m| m.get_fn(hash).map(|f| (f, m.id_raw())));
#[cfg(feature = "no_function")]
let func = None;

// Then check the global namespace
let func = func.or_else(|| {
self.global_modules
.iter()
.find_map(|m| m.get_fn(hash).map(|f| (f, m.id_raw())))
});

// Then check imported modules for global functions, then global sub-modules for global functions
#[cfg(not(feature = "no_module"))]
let func = func
.or_else(|| _global.get_qualified_fn(hash, true))
Expand Down
16 changes: 10 additions & 6 deletions src/func/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ impl Engine {
}

// Does a script-defined function exist?
///
/// # Note
///
/// If the scripted function is not found, this information is cached for future look-ups.
#[must_use]
pub(crate) fn has_script_fn(
&self,
Expand All @@ -219,27 +223,27 @@ impl Engine {
) -> bool {
let cache = caches.fn_resolution_cache_mut();

if let Some(result) = cache.map.get(&hash_script).map(Option::is_some) {
if let Some(result) = cache.dict.get(&hash_script).map(Option::is_some) {
return result;
}

// First check script-defined functions
let r = global.lib.iter().any(|m| m.contains_fn(hash_script))
let res = global.lib.iter().any(|m| m.contains_fn(hash_script))
// Then check the global namespace and packages
|| self.global_modules.iter().any(|m| m.contains_fn(hash_script));

#[cfg(not(feature = "no_module"))]
let r = r ||
let res = res ||
// Then check imported modules
global.contains_qualified_fn(hash_script)
// Then check sub-modules
|| self.global_sub_modules.values().any(|m| m.contains_qualified_fn(hash_script));

if !r && !cache.filter.is_absent_and_set(hash_script) {
if !res && !cache.filter.is_absent_and_set(hash_script) {
// Do not cache "one-hit wonders"
cache.map.insert(hash_script, None);
cache.dict.insert(hash_script, None);
}

r
res
}
}
2 changes: 1 addition & 1 deletion src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ fn optimize_combo_chain(expr: &mut Expr) {
Expr::Dot(mut x, opt, pos) => match x.lhs.take() {
#[cfg(not(feature = "no_index"))]
Expr::Index(x2, opt2, pos2) => (x, opt, pos, x2, opt2, pos2, Expr::Dot, Expr::Index),
Expr::Dot(x2, opt2, pos2) => (x, opt, pos, x2, opt2, pos2, Expr::Dot, Expr::Index),
Expr::Dot(x2, opt2, pos2) => (x, opt, pos, x2, opt2, pos2, Expr::Dot, Expr::Dot),
_ => unreachable!("combo chain expected"),
},
_ => unreachable!("combo chain expected"),
Expand Down
2 changes: 2 additions & 0 deletions tests/maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ fn test_map_indexing() {
}

assert_eq!(engine.eval::<INT>("let y = #{a: 1, b: 2, c: 3}; y.a = 5; y.a").unwrap(), 5);
assert_eq!(engine.eval::<INT>("let y = #{a: #{x:9, y:8, z:7}, b: 2, c: 3}; (y.a).z").unwrap(), 7);
assert_eq!(engine.eval::<INT>("let y = #{a: #{x:9, y:8, z:7}, b: 2, c: 3}; (y.a).z = 42; y.a.z").unwrap(), 42);

engine.run("let y = #{a: 1, b: 2, c: 3}; y.z").unwrap();

Expand Down
Loading