Skip to content

fix: prevent silent integer overflows in rate limiter and expression … #47

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.

Sign up for GitHub

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

Open
Pama-Lee wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: prevent silent integer overflows in rate limiter and expression … #47

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d6770dd
fix: prevent silent integer overflows in rate limiter and expression …
claude Mar 14, 2026
94631d3
feat: add 38 OPA-aligned built-in functions to expression engine
claude Mar 14, 2026
c2c6536
style: cargo fmt
Pama-Lee Mar 17, 2026
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
8 changes: 7 additions & 1 deletion crates/ordo-core/src/expr/compiler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ impl ExprCompiler {
#[inline]
fn alloc_reg(&mut self) -> u8 {
let reg = self.next_reg;
self.next_reg += 1;
self.next_reg = self.next_reg.checked_add(1).expect(
"register limit exceeded: expression requires more than 256 registers",
);
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow allocation of register 255 before overflow check

alloc_reg now panics when next_reg == 255 because checked_add(1) is evaluated before returning, so register index 255 is never usable. That is an off-by-one regression: the VM uses 8-bit register operands and a 256-slot register file ([Value; 256] in vm.rs), and this same commit still treats 255 function arguments as valid (args.len() < 256). Expressions that require the highest register (e.g. wide function calls) now panic even though they are representable.

Useful? React with 👍 / 👎.

reg
}

Expand Down Expand Up @@ -77,6 +79,7 @@ impl ExprCompiler {
return idx as u8;
}
let idx = self.compiled.constants.len();
assert!(idx < 256, "constant pool limit exceeded: expression has more than 256 unique constants");
self.compiled.constants.push(value);
idx as u8
}
Expand All @@ -87,6 +90,7 @@ impl ExprCompiler {
return idx as u8;
}
let idx = self.compiled.fields.len();
assert!(idx < 256, "field pool limit exceeded: expression references more than 256 unique fields");
self.compiled.fields.push(name.to_string());
idx as u8
}
Expand All @@ -97,6 +101,7 @@ impl ExprCompiler {
return idx as u8;
}
let idx = self.compiled.functions.len();
assert!(idx < 256, "function pool limit exceeded: expression references more than 256 unique functions");
self.compiled.functions.push(name.to_string());
idx as u8
}
Expand Down Expand Up @@ -166,6 +171,7 @@ impl ExprCompiler {
}

let func_idx = self.add_function(name);
assert!(args.len() < 256, "argument count limit exceeded: function call has more than 255 arguments");
// For Call: a=result, b=func_idx, c=arg_count
// Arguments are expected in registers [result_reg+1, result_reg+1+arg_count)
self.emit(Instruction::new(
Expand Down
7 changes: 5 additions & 2 deletions crates/ordo-server/src/rate_limiter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,17 @@ impl TokenBucket {
return;
}
let elapsed_ms = now_ms - last;
let tokens_to_add = (elapsed_ms as u128 * self.refill_rate as u128 / 1000) as u32;
// Clamp to capacity before casting to u32 to avoid truncation
let tokens_to_add = (elapsed_ms as u128 * self.refill_rate as u128 / 1000)
.min(self.capacity as u128) as u32;
if tokens_to_add == 0 {
return;
}

let mut current = self.tokens.load(Ordering::Relaxed);
loop {
let new_tokens = (current + tokens_to_add).min(self.capacity);
// Use saturating_add to prevent overflow before .min()
let new_tokens = current.saturating_add(tokens_to_add).min(self.capacity);
match self.tokens.compare_exchange(
current,
new_tokens,
Expand Down
Loading