mirror of
https://github.com/microsoft/regorus.git
synced 2026-08-05 02:16:11 +00:00
fix: harden regex builtins with compiled-size limit (#705)
Add a 100KB cap on compiled regex NFA size via RegexBuilder::size_limit() to block patterns that blow up in memory or CPU. Regex compilation now goes through a single helper (compile_regex_for_builtin) so the limit is enforced consistently across all regex builtins. While doing this, found and fixed a pre-existing bug: resource-limit errors (time, memory, instruction count) raised inside builtins were quietly swallowed to Undefined when strict_builtin_errors was off (the default). This is a problem because `not regex.match(...)` would see Undefined and flip to true -- silently wrong. The same issue now applies to the new regex size limit. Fixed by teaching the three error-absorption paths (interpreter builtin call, RVM builtin dispatch, and RVM rule-execution loop) to recognize LimitError and let it propagate instead of eating it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
c312e30372
commit
87f22a79ca
@@ -28,6 +28,9 @@ pub enum VmError {
|
||||
#[error("Execution exceeded memory limit (usage={usage} bytes, limit={limit} bytes, pc={pc})")]
|
||||
MemoryLimitExceeded { usage: u64, limit: u64, pc: usize },
|
||||
|
||||
#[error("Compiled regex exceeded size limit ({limit} bytes, pc={pc})")]
|
||||
RegexSizeLimitExceeded { limit: usize, pc: usize },
|
||||
|
||||
#[error("Literal index {index} out of bounds (pc={pc})")]
|
||||
LiteralIndexOutOfBounds { index: u16, pc: usize },
|
||||
|
||||
@@ -298,6 +301,32 @@ pub enum VmError {
|
||||
|
||||
impl From<anyhow::Error> for VmError {
|
||||
fn from(err: anyhow::Error) -> Self {
|
||||
// Preserve LimitError identity so that resource-limit violations are
|
||||
// never silently swallowed to Undefined in non-strict mode.
|
||||
// Note: pc is set to 0 because this conversion lacks instruction context.
|
||||
// The error message itself (which includes the limit value) provides
|
||||
// sufficient diagnostic information for users.
|
||||
if let Some(limit_err) = err.downcast_ref::<crate::LimitError>() {
|
||||
return match *limit_err {
|
||||
crate::LimitError::TimeLimitExceeded { elapsed, limit } => {
|
||||
VmError::TimeLimitExceeded {
|
||||
elapsed,
|
||||
limit,
|
||||
pc: 0,
|
||||
}
|
||||
}
|
||||
crate::LimitError::MemoryLimitExceeded { usage, limit } => {
|
||||
VmError::MemoryLimitExceeded {
|
||||
usage,
|
||||
limit,
|
||||
pc: 0,
|
||||
}
|
||||
}
|
||||
crate::LimitError::RegexSizeLimitExceeded { limit } => {
|
||||
VmError::RegexSizeLimitExceeded { limit, pc: 0 }
|
||||
}
|
||||
};
|
||||
}
|
||||
VmError::ArithmeticError {
|
||||
message: alloc::format!("{}", err),
|
||||
pc: 0,
|
||||
|
||||
@@ -570,7 +570,15 @@ impl RegoVM {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn handle_instruction_error(&mut self, _err: VmError, last_result: &mut Value) -> Result<bool> {
|
||||
fn handle_instruction_error(&mut self, err: VmError, last_result: &mut Value) -> Result<bool> {
|
||||
// Resource-limit errors must never be absorbed by rule evaluation.
|
||||
// They represent engine-level constraints, not rule-level failures.
|
||||
// Returning Ok(false) causes the caller to clear execution_stack and
|
||||
// propagate the error, terminating the evaluation entirely.
|
||||
if RegoVM::is_fatal_vm_error(&err) {
|
||||
return Ok(false);
|
||||
}
|
||||
|
||||
if let Some(frame) = self.execution_stack.pop() {
|
||||
match frame.kind {
|
||||
FrameKind::Rule(mut data) => {
|
||||
|
||||
+10
-1
@@ -122,7 +122,16 @@ impl RegoVM {
|
||||
self.strict_builtin_errors,
|
||||
) {
|
||||
Ok(value) => value,
|
||||
Err(_) if !self.strict_builtin_errors => Value::Undefined,
|
||||
// Resource-limit errors must always propagate, even in non-strict
|
||||
// mode, to prevent `not builtin(...)` from silently flipping to true.
|
||||
Err(e) if !self.strict_builtin_errors => {
|
||||
if e.downcast_ref::<crate::LimitError>().is_some() {
|
||||
self.dummy_exprs = dummy_exprs;
|
||||
self.cached_builtin_args = args;
|
||||
return Err(e.into());
|
||||
}
|
||||
Value::Undefined
|
||||
}
|
||||
Err(err) => {
|
||||
self.dummy_exprs = dummy_exprs;
|
||||
self.cached_builtin_args = args;
|
||||
|
||||
@@ -443,6 +443,9 @@ impl RegoVM {
|
||||
limit,
|
||||
pc: self.pc,
|
||||
},
|
||||
LimitError::RegexSizeLimitExceeded { limit } => {
|
||||
VmError::RegexSizeLimitExceeded { limit, pc: self.pc }
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -17,6 +17,36 @@ use super::execution_model::{
|
||||
use super::machine::RegoVM;
|
||||
|
||||
impl RegoVM {
|
||||
/// Returns true if the error represents a resource-limit violation that
|
||||
/// must never be silently absorbed by rule evaluation.
|
||||
pub(super) const fn is_fatal_vm_error(err: &VmError) -> bool {
|
||||
matches!(
|
||||
err,
|
||||
VmError::TimeLimitExceeded { .. }
|
||||
| VmError::MemoryLimitExceeded { .. }
|
||||
| VmError::RegexSizeLimitExceeded { .. }
|
||||
| VmError::InstructionLimitExceeded { .. }
|
||||
)
|
||||
}
|
||||
|
||||
/// Restore VM state that was swapped out for rule execution.
|
||||
/// Must be called before returning an error from `execute_rule_definitions_common`
|
||||
/// to avoid leaving the VM in an inconsistent state.
|
||||
fn restore_rule_state(
|
||||
&mut self,
|
||||
previous_loop_stack: &mut Vec<super::context::LoopContext>,
|
||||
previous_comprehension_stack: &mut Vec<super::context::ComprehensionContext>,
|
||||
) {
|
||||
if let Some(restored_registers) = self.register_stack.pop() {
|
||||
let mut current_register_window = Vec::default();
|
||||
mem::swap(&mut current_register_window, &mut self.registers);
|
||||
self.return_register_window(current_register_window);
|
||||
self.registers = restored_registers;
|
||||
}
|
||||
mem::swap(&mut self.loop_stack, previous_loop_stack);
|
||||
mem::swap(&mut self.comprehension_stack, previous_comprehension_stack);
|
||||
}
|
||||
|
||||
pub(super) fn execute_rule_definitions_common(
|
||||
&mut self,
|
||||
rule_definitions: &[Vec<u32>],
|
||||
@@ -79,6 +109,13 @@ impl RegoVM {
|
||||
{
|
||||
match self.jump_to(destructuring_entry_point) {
|
||||
Ok(_result) => {}
|
||||
Err(e) if Self::is_fatal_vm_error(&e) => {
|
||||
self.restore_rule_state(
|
||||
&mut previous_loop_stack,
|
||||
&mut previous_comprehension_stack,
|
||||
);
|
||||
return Err(e);
|
||||
}
|
||||
Err(_e) => {
|
||||
continue 'outer;
|
||||
}
|
||||
@@ -111,6 +148,13 @@ impl RegoVM {
|
||||
// are treated as else-branches and must not be evaluated.
|
||||
break;
|
||||
}
|
||||
Err(e) if Self::is_fatal_vm_error(&e) => {
|
||||
self.restore_rule_state(
|
||||
&mut previous_loop_stack,
|
||||
&mut previous_comprehension_stack,
|
||||
);
|
||||
return Err(e);
|
||||
}
|
||||
Err(_e) => {}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user