Fix - JIT second level defence (#554)

* emit_internal_call() actually has an user provided target parameter. * Make sure the first few instructions generated get some address randomization as well. * Passively limits the maximum output size. * Makes linter happy.
solana-labs · Apr 2, 2024 · 40a30d8 · 40a30d8
1 parent 179a0f9
commit 40a30d8
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 8 deletions.
diff --git a/src/aligned_memory.rs b/src/aligned_memory.rs
@@ -212,9 +212,9 @@ pub fn is_memory_aligned(ptr: usize, align: usize) -> bool {
         .unwrap_or(false)
 }
 
+#[allow(clippy::arithmetic_side_effects)]
 #[cfg(test)]
 mod tests {
-    #![allow(clippy::arithmetic_side_effects)]
     use {super::*, std::io::Write};
 
     fn do_test<const ALIGN: usize>() {

diff --git a/src/elf.rs b/src/elf.rs
@@ -1176,7 +1176,6 @@ pub(crate) fn get_ro_region(ro_section: &Section, elf: &[u8]) -> MemoryRegion {
 mod test {
     use super::*;
     use crate::{
-        ebpf,
         elf_parser::{
             // FIXME consts::{ELFCLASS32, ELFDATA2MSB, ET_REL},
             consts::{ELFCLASS32, ELFDATA2MSB, ET_REL},

diff --git a/src/jit.rs b/src/jit.rs
@@ -347,6 +347,8 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
         if config.instruction_meter_checkpoint_distance != 0 {
             code_length_estimate += pc / config.instruction_meter_checkpoint_distance * MACHINE_CODE_PER_INSTRUCTION_METER_CHECKPOINT;
         }
+        // Relative jump destinations limit the maximum output size
+        debug_assert!(code_length_estimate < (i32::MAX as usize));
 
         let runtime_environment_key = get_runtime_environment_key();
         let mut diversification_rng = SmallRng::from_rng(rand::thread_rng()).map_err(|_| EbpfError::JitNotCompiled)?;
@@ -373,6 +375,14 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
     pub fn compile(mut self) -> Result<JitProgram, EbpfError> {
         let text_section_base = self.result.text_section.as_ptr();
 
+        // Randomized padding at the start before random intervals begin
+        if self.config.noop_instruction_rate != 0 {
+            for _ in 0..self.diversification_rng.gen_range(0..self.config.noop_instruction_rate) {
+                // X86Instruction::noop().emit(self)?;
+                self.emit::<u8>(0x90);
+            }
+        }
+
         self.emit_subroutines();
 
         while self.pc * ebpf::INSN_SIZE < self.program.len() {
@@ -654,7 +664,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
 
                     if internal {
                         if let Some((_function_name, target_pc)) = self.executable.get_function_registry().lookup_by_key(insn.imm as u32) {
-                            self.emit_internal_call(Value::Constant64(target_pc as i64, false));
+                            self.emit_internal_call(Value::Constant64(target_pc as i64, true));
                             resolved = true;
                         }
                     }
@@ -1023,9 +1033,13 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
                 self.emit_ins(X86Instruction::call_reg(REGISTER_OTHER_SCRATCH, None)); // callq *REGISTER_OTHER_SCRATCH
             },
             Value::Constant64(target_pc, user_provided) => {
-                debug_assert!(!user_provided);
+                debug_assert!(user_provided);
                 self.emit_validate_and_profile_instruction_count(false, Some(target_pc as usize));
-                self.emit_ins(X86Instruction::load_immediate(OperandSize::S64, REGISTER_SCRATCH, target_pc));
+                if user_provided && self.should_sanitize_constant(target_pc) {
+                    self.emit_sanitized_load_immediate(OperandSize::S64, REGISTER_SCRATCH, target_pc);
+                } else {
+                    self.emit_ins(X86Instruction::load_immediate(OperandSize::S64, REGISTER_SCRATCH, target_pc));
+                }
                 let jump_offset = self.relative_to_target_pc(target_pc as usize, 5);
                 self.emit_ins(X86Instruction::call_immediate(jump_offset));
             },

diff --git a/src/lib.rs b/src/lib.rs
@@ -56,6 +56,7 @@ trait ErrCheckedArithmetic: Sized {
     fn err_checked_add(self, other: Self) -> Result<Self, ArithmeticOverflow>;
     fn err_checked_sub(self, other: Self) -> Result<Self, ArithmeticOverflow>;
     fn err_checked_mul(self, other: Self) -> Result<Self, ArithmeticOverflow>;
+    #[allow(dead_code)]
     fn err_checked_div(self, other: Self) -> Result<Self, ArithmeticOverflow>;
 }
 struct ArithmeticOverflow;

diff --git a/src/program.rs b/src/program.rs
@@ -345,7 +345,7 @@ macro_rules! declare_builtin_function {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{program::BuiltinFunction, syscalls, vm::TestContextObject};
+    use crate::{syscalls, vm::TestContextObject};
 
     #[test]
     fn test_builtin_program_eq() {

diff --git a/src/static_analysis.rs b/src/static_analysis.rs
@@ -202,7 +202,11 @@ impl<'a> Analysis<'a> {
     fn link_cfg_edges(&mut self, cfg_edges: Vec<(usize, Vec<usize>)>, both_directions: bool) {
         for (source, destinations) in cfg_edges {
             if both_directions {
-                self.cfg_nodes.get_mut(&source).unwrap().destinations = destinations.clone();
+                self.cfg_nodes
+                    .get_mut(&source)
+                    .unwrap()
+                    .destinations
+                    .clone_from(&destinations);
             }
             for destination in &destinations {
                 self.cfg_nodes
@@ -350,7 +354,7 @@ impl<'a> Analysis<'a> {
                 }
                 if let Some(next_cfg_edge) = cfg_edge_iter.peek() {
                     if *next_cfg_edge.0 <= cfg_node_end {
-                        cfg_node.destinations = next_cfg_edge.1 .1.clone();
+                        cfg_node.destinations.clone_from(&next_cfg_edge.1 .1);
                         cfg_edge_iter.next();
                         continue;
                     }