Makes #212 configurable for feature gate. (#213)

solana-labs · Aug 16, 2021 · d0b0a16 · d0b0a16
1 parent 64ff80c
commit d0b0a16
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 29 deletions.
diff --git a/src/verifier.rs b/src/verifier.rs
@@ -234,9 +234,9 @@ pub fn check(prog: &[u8], config: &Config) -> Result<(), VerifierError> {
             ebpf::OR32_REG   => {},
             ebpf::AND32_IMM  => {},
             ebpf::AND32_REG  => {},
-            ebpf::LSH32_IMM  => { check_imm_shift(&insn, insn_ptr, 32)?; },
+            ebpf::LSH32_IMM  => { check_imm_shift(&insn, insn_ptr, if config.verify_shift32_imm { 32 } else { 64 })?; },
             ebpf::LSH32_REG  => {},
-            ebpf::RSH32_IMM  => { check_imm_shift(&insn, insn_ptr, 32)?; },
+            ebpf::RSH32_IMM  => { check_imm_shift(&insn, insn_ptr, if config.verify_shift32_imm { 32 } else { 64 })?; },
             ebpf::RSH32_REG  => {},
             ebpf::NEG32      => {},
             ebpf::MOD32_IMM  => { check_imm_nonzero(&insn, insn_ptr)?; },
@@ -245,7 +245,7 @@ pub fn check(prog: &[u8], config: &Config) -> Result<(), VerifierError> {
             ebpf::XOR32_REG  => {},
             ebpf::MOV32_IMM  => {},
             ebpf::MOV32_REG  => {},
-            ebpf::ARSH32_IMM => { check_imm_shift(&insn, insn_ptr, 32)?; },
+            ebpf::ARSH32_IMM => { check_imm_shift(&insn, insn_ptr, if config.verify_shift32_imm { 32 } else { 64 })?; },
             ebpf::ARSH32_REG => {},
             ebpf::LE         => { check_imm_endian(&insn, insn_ptr)?; },
             ebpf::BE         => { check_imm_endian(&insn, insn_ptr)?; },

diff --git a/src/vm.rs b/src/vm.rs
@@ -197,6 +197,8 @@ pub struct Config {
     pub encrypt_environment_registers: bool,
     /// Feature flag for the MUL64_IMM != 0 verification check
     pub verify_mul64_imm_nonzero: bool,
+    /// Feature flag for the SHIFT_IMM >= 32 verification check
+    pub verify_shift32_imm: bool,
 }
 impl Default for Config {
     fn default() -> Self {
@@ -212,6 +214,7 @@ impl Default for Config {
             sanitize_user_provided_values: true,
             encrypt_environment_registers: true,
             verify_mul64_imm_nonzero: false,
+            verify_shift32_imm: false,
         }
     }
 }

diff --git a/tests/ubpf_verifier.rs b/tests/ubpf_verifier.rs
@@ -215,43 +215,48 @@ fn test_verifier_err_write_r10() {
 fn test_verifier_err_all_shift_overflows() {
     let testcases = [
         // lsh32_imm
-        ("lsh32 r0, 32", "ShiftWithOverflow(32, 32, 29)"),
-        ("lsh32 r0, 42", "ShiftWithOverflow(42, 32, 29)"),
+        ("lsh32 r0, 16", Ok(())),
+        ("lsh32 r0, 32", Err("ShiftWithOverflow(32, 32, 29)")),
+        ("lsh32 r0, 64", Err("ShiftWithOverflow(64, 32, 29)")),
         // rsh32_imm
-        ("rsh32 r0, 32", "ShiftWithOverflow(32, 32, 29)"),
-        ("rsh32 r0, 42", "ShiftWithOverflow(42, 32, 29)"),
+        ("rsh32 r0, 16", Ok(())),
+        ("rsh32 r0, 32", Err("ShiftWithOverflow(32, 32, 29)")),
+        ("rsh32 r0, 64", Err("ShiftWithOverflow(64, 32, 29)")),
         // arsh32_imm
-        ("arsh32 r0, 32", "ShiftWithOverflow(32, 32, 29)"),
-        ("arsh32 r0, 42", "ShiftWithOverflow(42, 32, 29)"),
+        ("arsh32 r0, 16", Ok(())),
+        ("arsh32 r0, 32", Err("ShiftWithOverflow(32, 32, 29)")),
+        ("arsh32 r0, 64", Err("ShiftWithOverflow(64, 32, 29)")),
         // lsh64_imm
-        ("lsh64 r0, 64", "ShiftWithOverflow(64, 64, 29)"),
-        ("lsh64 r0, 250", "ShiftWithOverflow(250, 64, 29)"),
+        ("lsh64 r0, 32", Ok(())),
+        ("lsh64 r0, 64", Err("ShiftWithOverflow(64, 64, 29)")),
         // rsh64_imm
-        ("rsh64 r0, 64", "ShiftWithOverflow(64, 64, 29)"),
-        ("rsh64 r0, 250", "ShiftWithOverflow(250, 64, 29)"),
+        ("rsh64 r0, 32", Ok(())),
+        ("rsh64 r0, 64", Err("ShiftWithOverflow(64, 64, 29)")),
         // arsh64_imm
-        ("arsh64 r0, 64", "ShiftWithOverflow(64, 64, 29)"),
-        ("arsh64 r0, 250", "ShiftWithOverflow(250, 64, 29)"),
+        ("arsh64 r0, 32", Ok(())),
+        ("arsh64 r0, 64", Err("ShiftWithOverflow(64, 64, 29)")),
     ];
 
-    for (overflowing_instruction, overflow_msg) in testcases {
-        let code = format!("\n{}\nexit", overflowing_instruction);
-        let expected_err = format!("Executable constructor VerifierError({})", overflow_msg);
-
+    for (overflowing_instruction, expected) in testcases {
+        let assembly = format!("\n{}\nexit", overflowing_instruction);
         let result = assemble::<UserError, TestInstructionMeter>(
-            &code,
+            &assembly,
             Some(check),
-            Config::default(),
+            Config {
+                verify_shift32_imm: true,
+                ..Config::default()
+            },
             SyscallRegistry::default(),
         );
-
-        match result {
-            Err(err) => {
-                assert_eq!(err, expected_err);
-            }
-            _ => {
-                panic!("Incorrect test result");
-            }
+        match expected {
+            Ok(()) => assert!(result.is_ok()),
+            Err(overflow_msg) => match result {
+                Err(err) => assert_eq!(
+                    err,
+                    format!("Executable constructor VerifierError({})", overflow_msg),
+                ),
+                _ => panic!("Expected error"),
+            },
         }
     }
 }