Skip to content

Commit 740db8a

Browse files
anakrishanakrish
authored
chore: Harden RVM implementation (microsoft#537)
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
1 parent d626f75 commit 740db8a

16 files changed

Lines changed: 1598 additions & 953 deletions

‎src/rvm/instructions/types.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ pub enum LiteralOrRegister {
1515

1616
/// Loop execution modes for different Rego iteration constructs
1717
#[repr(C)]
18-
#[derive(Debug, Clone, Serialize, Deserialize)]
18+
#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
1919
pub enum LoopMode {
2020
/// Any quantification: some x in arr, x := arr[_], etc.
2121
/// Succeeds if ANY iteration succeeds, exits early on first success

‎src/rvm/program/rule_tree.rs‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,13 @@ impl Program {
7070
match *rule_value {
7171
Value::Number(_) => {
7272
if data_value != &Value::Undefined {
73-
return Err(crate::rvm::vm::VmError::RuleDataConflict(format!(
74-
"Conflict: rule defines path '{}' but data also provides this path",
75-
current_path.join("."),
76-
)));
73+
return Err(crate::rvm::vm::VmError::RuleDataConflict {
74+
message: format!(
75+
"Conflict: rule defines path '{}' but data also provides this path",
76+
current_path.join("."),
77+
),
78+
pc: 0,
79+
});
7780
}
7881
}
7982
Value::Object(_) => {
@@ -84,17 +87,23 @@ impl Program {
8487
current_path,
8588
)?;
8689
} else if data_value != &Value::Undefined {
87-
return Err(crate::rvm::vm::VmError::RuleDataConflict(format!(
88-
"Conflict: rule defines subpaths under '{}' but data provides a non-object value at this path",
89-
current_path.join("."),
90-
)));
90+
return Err(crate::rvm::vm::VmError::RuleDataConflict {
91+
message: format!(
92+
"Conflict: rule defines subpaths under '{}' but data provides a non-object value at this path",
93+
current_path.join("."),
94+
),
95+
pc: 0,
96+
});
9197
}
9298
}
9399
_ => {
94-
return Err(crate::rvm::vm::VmError::RuleDataConflict(format!(
95-
"Invalid rule tree structure at path '{}'",
96-
current_path.join("."),
97-
)));
100+
return Err(crate::rvm::vm::VmError::RuleDataConflict {
101+
message: format!(
102+
"Invalid rule tree structure at path '{}'",
103+
current_path.join("."),
104+
),
105+
pc: 0,
106+
});
98107
}
99108
}
100109

@@ -103,9 +112,10 @@ impl Program {
103112
}
104113
}
105114
_ => {
106-
return Err(crate::rvm::vm::VmError::RuleDataConflict(
107-
"Rule tree root must be an object".to_string(),
108-
));
115+
return Err(crate::rvm::vm::VmError::RuleDataConflict {
116+
message: "Rule tree root must be an object".to_string(),
117+
pc: 0,
118+
});
109119
}
110120
}
111121

‎src/rvm/tests/vm.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,7 @@ mod tests {
702702
VmError::HostAwaitResponseMissing {
703703
dest,
704704
identifier: identifier.clone(),
705+
pc: 0,
705706
}
706707
)
707708
})?;
@@ -712,6 +713,7 @@ mod tests {
712713
VmError::HostAwaitResponseMissing {
713714
dest,
714715
identifier: identifier.clone(),
716+
pc: 0,
715717
}
716718
)
717719
})?;
@@ -722,6 +724,7 @@ mod tests {
722724
VmError::HostAwaitResponseMissing {
723725
dest,
724726
identifier: identifier.clone(),
727+
pc: 0,
725728
}
726729
)
727730
})?;

‎src/rvm/vm/arithmetic.rs‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
1-
#![allow(
2-
clippy::unused_self,
3-
clippy::missing_const_for_fn,
4-
clippy::unseparated_literal_suffix,
5-
clippy::pattern_type_mismatch
6-
)]
71
// Copyright (c) Microsoft Corporation.
82
// Licensed under the MIT License.
3+
#![allow(clippy::pattern_type_mismatch)]
94

105
use alloc::collections::BTreeSet;
116

@@ -23,6 +18,7 @@ impl RegoVM {
2318
_ => Err(VmError::InvalidAddition {
2419
left: a.clone(),
2520
right: b.clone(),
21+
pc: self.pc,
2622
}),
2723
}
2824
}
@@ -38,6 +34,7 @@ impl RegoVM {
3834
_ => Err(VmError::InvalidSubtraction {
3935
left: a.clone(),
4036
right: b.clone(),
37+
pc: self.pc,
4138
}),
4239
}
4340
}
@@ -49,6 +46,7 @@ impl RegoVM {
4946
_ => Err(VmError::InvalidMultiplication {
5047
left: a.clone(),
5148
right: b.clone(),
49+
pc: self.pc,
5250
}),
5351
}
5452
}
@@ -57,11 +55,12 @@ impl RegoVM {
5755
pub(super) fn div_values(&self, a: &Value, b: &Value) -> Result<Value> {
5856
match (a, b) {
5957
(Value::Number(x), Value::Number(y)) => {
60-
if *y == Number::from(0u64) {
58+
if *y == Number::from(0_u64) {
6159
if self.strict_builtin_errors {
6260
return Err(VmError::InvalidDivision {
6361
left: a.clone(),
6462
right: b.clone(),
63+
pc: self.pc,
6564
});
6665
}
6766
return Ok(Value::Undefined);
@@ -72,6 +71,7 @@ impl RegoVM {
7271
_ => Err(VmError::InvalidDivision {
7372
left: a.clone(),
7473
right: b.clone(),
74+
pc: self.pc,
7575
}),
7676
}
7777
}
@@ -80,30 +80,36 @@ impl RegoVM {
8080
pub(super) fn mod_values(&self, a: &Value, b: &Value) -> Result<Value> {
8181
match (a, b) {
8282
(Value::Number(x), Value::Number(y)) => {
83-
if *y == Number::from(0u64) {
83+
if *y == Number::from(0_u64) {
8484
if self.strict_builtin_errors {
8585
return Err(VmError::InvalidModulo {
8686
left: a.clone(),
8787
right: b.clone(),
88+
pc: self.pc,
8889
});
8990
}
9091
return Ok(Value::Undefined);
9192
}
9293

9394
if !x.is_integer() || !y.is_integer() {
94-
return Err(VmError::ModuloOnFloat);
95+
return Err(VmError::ModuloOnFloat {
96+
left: a.clone(),
97+
right: b.clone(),
98+
pc: self.pc,
99+
});
95100
}
96101

97102
Ok(Value::from(x.clone().modulo(y)?))
98103
}
99104
_ => Err(VmError::InvalidModulo {
100105
left: a.clone(),
101106
right: b.clone(),
107+
pc: self.pc,
102108
}),
103109
}
104110
}
105111

106-
pub(super) fn to_bool(&self, value: &Value) -> Option<bool> {
112+
pub(super) const fn to_bool(&self, value: &Value) -> Option<bool> {
107113
match value {
108114
Value::Bool(b) => Some(*b),
109115
Value::Null if !self.strict_builtin_errors => Some(true),

0 commit comments

Comments
 (0)