From 44f33fc821203e03cc3e41cc2d992a478b0f8aad Mon Sep 17 00:00:00 2001 From: Lucas Date: Thu, 5 Sep 2024 12:18:09 -0300 Subject: [PATCH] Check if syscall is valid during verification --- src/elf.rs | 1 + src/verifier.rs | 20 ++++++++++++------- tests/verifier.rs | 51 +++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/elf.rs b/src/elf.rs index c5b531d4d..9a16affa4 100644 --- a/src/elf.rs +++ b/src/elf.rs @@ -315,6 +315,7 @@ impl Executable { self.get_config(), self.get_sbpf_version(), self.get_function_registry(), + self.loader.get_function_registry(), )?; Ok(()) } diff --git a/src/verifier.rs b/src/verifier.rs index c4f774bad..e8b098788 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -14,8 +14,8 @@ use crate::{ ebpf, - program::{FunctionRegistry, SBPFVersion}, - vm::Config, + program::{BuiltinFunction, FunctionRegistry, SBPFVersion}, + vm::{Config, ContextObject}, }; use thiserror::Error; @@ -85,11 +85,12 @@ pub trait Verifier { /// - Unknown instructions. /// - Bad formed instruction. /// - Unknown eBPF syscall index. - fn verify( + fn verify( prog: &[u8], config: &Config, sbpf_version: &SBPFVersion, function_registry: &FunctionRegistry, + sysvar_registry: &FunctionRegistry>, ) -> Result<(), VerifierError>; } @@ -153,10 +154,14 @@ fn check_jmp_offset( Ok(()) } -fn check_call_target( +fn check_call_target( key: u32, - function_registry: &FunctionRegistry, -) -> Result<(), VerifierError> { + function_registry: &FunctionRegistry, +) -> Result<(), VerifierError> +where + T: Copy, + T: PartialEq, +{ function_registry .lookup_by_key(key) .map(|_| ()) @@ -216,7 +221,7 @@ pub struct RequisiteVerifier {} impl Verifier for RequisiteVerifier { /// Check the program against the verifier's rules #[rustfmt::skip] - fn verify(prog: &[u8], config: &Config, sbpf_version: &SBPFVersion, function_registry: &FunctionRegistry) -> Result<(), VerifierError> { + fn verify(prog: &[u8], config: &Config, sbpf_version: &SBPFVersion, function_registry: &FunctionRegistry, sysvar_registry: &FunctionRegistry>) -> Result<(), VerifierError> { check_prog_len(prog)?; let program_range = 0..prog.len() / ebpf::INSN_SIZE; @@ -371,6 +376,7 @@ impl Verifier for RequisiteVerifier { ebpf::JSLE_IMM => { check_jmp_offset(prog, insn_ptr, &function_range)?; }, ebpf::JSLE_REG => { check_jmp_offset(prog, insn_ptr, &function_range)?; }, ebpf::CALL_IMM if sbpf_version.static_syscalls() && insn.src != 0 => { check_call_target(insn.imm as u32, function_registry)?; }, + ebpf::CALL_IMM if sbpf_version.static_syscalls() && insn.src == 0 => { check_call_target(insn.imm as u32, sysvar_registry)?; }, ebpf::CALL_IMM => {}, ebpf::CALL_REG => { check_callx_register(&insn, insn_ptr, config, sbpf_version)?; }, ebpf::EXIT => {}, diff --git a/tests/verifier.rs b/tests/verifier.rs index b7fb9c232..6347d9cd2 100644 --- a/tests/verifier.rs +++ b/tests/verifier.rs @@ -26,9 +26,10 @@ use solana_rbpf::{ assembler::assemble, ebpf, elf::Executable, - program::{BuiltinProgram, FunctionRegistry, SBPFVersion}, + program::{BuiltinFunction, BuiltinProgram, FunctionRegistry, SBPFVersion}, + syscalls, verifier::{RequisiteVerifier, Verifier, VerifierError}, - vm::{Config, TestContextObject}, + vm::{Config, ContextObject, TestContextObject}, }; use std::sync::Arc; use test_utils::{assert_error, create_vm}; @@ -43,11 +44,12 @@ pub enum VerifierTestError { struct TautologyVerifier {} impl Verifier for TautologyVerifier { - fn verify( + fn verify( _prog: &[u8], _config: &Config, _sbpf_version: &SBPFVersion, _function_registry: &FunctionRegistry, + _syvar_registry: &FunctionRegistry>, ) -> std::result::Result<(), VerifierError> { Ok(()) } @@ -55,11 +57,12 @@ impl Verifier for TautologyVerifier { struct ContradictionVerifier {} impl Verifier for ContradictionVerifier { - fn verify( + fn verify( _prog: &[u8], _config: &Config, _sbpf_version: &SBPFVersion, _function_registry: &FunctionRegistry, + _sysvar_registry: &FunctionRegistry>, ) -> std::result::Result<(), VerifierError> { Err(VerifierError::NoProgram) } @@ -299,6 +302,46 @@ fn test_verifier_err_unknown_opcode() { executable.verify::().unwrap(); } +#[test] +#[should_panic(expected = "InvalidFunction(1811268606)")] +fn test_verifier_unknown_sycall() { + let prog = &[ + 0x85, 0x00, 0x00, 0x00, 0xfe, 0xc3, 0xf5, 0x6b, // call 0x6bf5c3fe + 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // exit + ]; + let executable = Executable::::from_text_bytes( + prog, + Arc::new(BuiltinProgram::new_mock()), + SBPFVersion::V2, + FunctionRegistry::default(), + ) + .unwrap(); + executable.verify::().unwrap(); +} + +#[test] +fn test_verifier_known_syscall() { + let prog = &[ + 0x85, 0x00, 0x00, 0x00, 0xfe, 0xc3, 0xf5, 0x6b, // call 0x6bf5c3fe + 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // exit + ]; + let mut function_registry = FunctionRegistry::>::default(); + function_registry + .register_function(0x6bf5c3fe, b"my_syscall", syscalls::SyscallString::vm) + .unwrap(); + let executable = Executable::::from_text_bytes( + prog, + Arc::new(BuiltinProgram::new_loader( + Config::default(), + function_registry, + )), + SBPFVersion::V2, + FunctionRegistry::default(), + ) + .unwrap(); + executable.verify::().unwrap(); +} + #[test] #[should_panic(expected = "CannotWriteR10(0)")] fn test_verifier_err_write_r10() {