diff --git a/src/cwe_checker_lib/src/analysis/backward_interprocedural_fixpoint/tests.rs b/src/cwe_checker_lib/src/analysis/backward_interprocedural_fixpoint/tests.rs index 57095e90a..edc038d08 100644 --- a/src/cwe_checker_lib/src/analysis/backward_interprocedural_fixpoint/tests.rs +++ b/src/cwe_checker_lib/src/analysis/backward_interprocedural_fixpoint/tests.rs @@ -31,21 +31,17 @@ fn mock_program() -> Term { tid: Tid::new("jump"), term: jmp, }; + let mut blk = Blk::default(); + blk.add_defs(vec![def_term1]).add_jumps(vec![call_term]); let sub1_blk1 = Term { tid: Tid::new("sub1_blk1"), - term: Blk { - defs: vec![def_term1], - jmps: vec![call_term], - indirect_jmp_targets: Vec::new(), - }, + term: blk, }; + let mut blk = Blk::default(); + blk.add_defs(vec![def_term5]).add_jumps(vec![jmp_term]); let sub1_blk2 = Term { tid: Tid::new("sub1_blk2"), - term: Blk { - defs: vec![def_term5], - jmps: vec![jmp_term], - indirect_jmp_targets: Vec::new(), - }, + term: blk, }; let sub1 = Term { tid: Tid::new("sub1"), @@ -63,21 +59,18 @@ fn mock_program() -> Term { tid: Tid::new("jump2"), term: Jmp::Branch(Tid::new("sub2_blk2")), }; + let mut blk = Blk::default(); + blk.add_defs(vec![def_term2, def_term3]) + .add_jumps(vec![cond_jump_term, jump_term_2]); let sub2_blk1 = Term { tid: Tid::new("sub2_blk1"), - term: Blk { - defs: vec![def_term2, def_term3], - jmps: vec![cond_jump_term, jump_term_2], - indirect_jmp_targets: Vec::new(), - }, + term: blk, }; + let mut blk = Blk::default(); + blk.add_defs(vec![def_term4]).add_jumps(vec![return_term]); let sub2_blk2 = Term { tid: Tid::new("sub2_blk2"), - term: Blk { - defs: vec![def_term4], - jmps: vec![return_term], - indirect_jmp_targets: Vec::new(), - }, + term: blk, }; let sub2 = Term { tid: Tid::new("sub2"), diff --git a/src/cwe_checker_lib/src/analysis/graph.rs b/src/cwe_checker_lib/src/analysis/graph.rs index f531942a6..0c38ecdb4 100644 --- a/src/cwe_checker_lib/src/analysis/graph.rs +++ b/src/cwe_checker_lib/src/analysis/graph.rs @@ -337,10 +337,11 @@ impl<'a> GraphBuilder<'a> { } } - /// Read in target hints for indirect intraprocedural jumps from the source block - /// and add intraprocedural jump edges for them to the graph. + /// Read in target hints for indirect intraprocedural jumps from the source + /// block and add intraprocedural jump edges for them to the graph. /// - /// The function assumes (but does not check) that the `jump` is an intraprocedural indirect jump. + /// The function assumes (but does not check) that the `jump` is an + /// intraprocedural indirect jump. fn add_indirect_jumps( &mut self, source: NodeIndex, @@ -351,8 +352,10 @@ impl<'a> GraphBuilder<'a> { Node::BlkEnd(source_block, _) => source_block, _ => panic!(), }; - for target_tid in source_block.term.indirect_jmp_targets.iter() { - self.add_intraprocedural_edge(source, target_tid, jump, untaken_conditional); + if let Some(indirect_jump_targets) = source_block.ind_jump_targets() { + for target_tid in indirect_jump_targets { + self.add_intraprocedural_edge(source, target_tid, jump, untaken_conditional); + } } } @@ -684,21 +687,17 @@ mod tests { tid: Tid::new("jump"), term: jmp, }; + let mut blk = Blk::default(); + blk.add_jumps(vec![call_term]); let sub1_blk1 = Term { tid: Tid::new("sub1_blk1"), - term: Blk { - defs: Vec::new(), - jmps: vec![call_term], - indirect_jmp_targets: Vec::new(), - }, + term: blk, }; + let mut blk = Blk::default(); + blk.add_jumps(vec![jmp_term]); let sub1_blk2 = Term { tid: Tid::new("sub1_blk2"), - term: Blk { - defs: Vec::new(), - jmps: vec![jmp_term], - indirect_jmp_targets: Vec::new(), - }, + term: blk, }; let sub1 = Term { tid: Tid::new("sub1"), @@ -716,21 +715,17 @@ mod tests { tid: Tid::new("jump2"), term: Jmp::Branch(Tid::new("sub2_blk2")), }; + let mut blk = Blk::default(); + blk.add_jumps(vec![cond_jump_term, jump_term_2]); let sub2_blk1 = Term { tid: Tid::new("sub2_blk1"), - term: Blk { - defs: Vec::new(), - jmps: vec![cond_jump_term, jump_term_2], - indirect_jmp_targets: Vec::new(), - }, + term: blk, }; + let mut blk = Blk::default(); + blk.add_jumps(vec![return_term]); let sub2_blk2 = Term { tid: Tid::new("sub2_blk2"), - term: Blk { - defs: Vec::new(), - jmps: vec![return_term], - indirect_jmp_targets: Vec::new(), - }, + term: blk, }; let sub2 = Term { tid: Tid::new("sub2"), @@ -765,13 +760,12 @@ mod tests { }; let mut blk_tid = Tid::new("blk_00001000"); blk_tid.set_address("00001000"); + let mut blk = Blk::default(); + blk.add_jumps(vec![indirect_jmp_term]) + .set_ind_jump_targets(vec![blk_tid.clone()]); let blk_term = Term { - tid: blk_tid.clone(), - term: Blk { - defs: Vec::new(), - jmps: vec![indirect_jmp_term], - indirect_jmp_targets: vec![blk_tid], - }, + tid: blk_tid, + term: blk, }; let sub_term = Term { tid: Tid::new("sub"), diff --git a/src/cwe_checker_lib/src/analysis/graph/intraprocedural_cfg.rs b/src/cwe_checker_lib/src/analysis/graph/intraprocedural_cfg.rs index 6ca56dfdc..447d14e53 100644 --- a/src/cwe_checker_lib/src/analysis/graph/intraprocedural_cfg.rs +++ b/src/cwe_checker_lib/src/analysis/graph/intraprocedural_cfg.rs @@ -185,13 +185,13 @@ impl<'a> IntraproceduralCfgBuilder<'a> { } // Indirect branches. Jmp::BranchInd(_) => { - for end_node_idx in b - .indirect_jmp_targets - .iter() - .map(|t| self.blk_tid_to_idx_map.get(t).unwrap().0) - { - self.graph - .add_edge(start_node_idx, end_node_idx, Edge::Jump(j, None)); + if let Some(indirect_jump_targets) = b.ind_jump_targets() { + for end_node_idx in + indirect_jump_targets.map(|t| self.blk_tid_to_idx_map.get(t).unwrap().0) + { + self.graph + .add_edge(start_node_idx, end_node_idx, Edge::Jump(j, None)); + } } } // No interprocedural edges. diff --git a/src/cwe_checker_lib/src/intermediate_representation/blk.rs b/src/cwe_checker_lib/src/intermediate_representation/blk.rs index b20dfdb7a..ef056d82b 100644 --- a/src/cwe_checker_lib/src/intermediate_representation/blk.rs +++ b/src/cwe_checker_lib/src/intermediate_representation/blk.rs @@ -1,5 +1,6 @@ use super::*; use crate::utils::log::LogMessage; +use std::ops::Deref; use std::{collections::HashSet, fmt}; /// A basic block is a sequence of `Def` instructions followed by up to two @@ -36,12 +37,167 @@ pub struct Blk { pub defs: Vec>, /// The `Jmp` instructions of the basic block pub jmps: Vec>, - /// If the basic block contains an indirect jump, - /// this field contains possible jump target addresses for the jump. + /// Iff the basic block ends in an indirect jump or call, this field + /// contains possible target addresses. /// - /// Note that possible targets of indirect calls are *not* contained, - /// i.e., only intraprocedural jump targets are contained in this field. - pub indirect_jmp_targets: Vec, + /// In general, this list is neither complete nor sound, i.e., it may + /// contain targets that are infeasible at runtime and miss targets that may + /// be observed. + indirect_control_flow_targets: Option>, +} + +/// Possible targets of an indirect control flow transfer. +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)] +pub enum IndirectCfTargets { + /// An indirect call. + Call(IndirectCallTargets), + /// An indirect jump. + Jump(IndirectJumpTargets), +} + +impl IndirectCfTargets { + /// Returns an iterator over the possible targets of the indirect jump at + /// the end of this block. + pub fn iter_ind_jump_targets(&self) -> Option> { + match self { + IndirectCfTargets::Jump(ts) => Some(ts.as_ref().iter()), + _ => None, + } + } + + /// Returns an iterator over the possible targets of the indirect jump at + /// the end of this block. + pub fn iter_ind_jump_targets_mut(&mut self) -> Option> { + match self { + IndirectCfTargets::Jump(ts) => Some(ts.as_mut().iter_mut()), + _ => None, + } + } + + /// Returns an iterator over the possible targets of the indirect call at + /// the end of this block. + pub fn iter_ind_call_targets(&self) -> Option> { + match self { + IndirectCfTargets::Call(x) => Some(x.as_ref().iter()), + _ => None, + } + } + + /// Returns an iterator over the possible targets of the indirect call at + /// the end of this block. + pub fn iter_ind_call_targets_mut(&mut self) -> Option> { + match self { + IndirectCfTargets::Call(x) => Some(x.as_mut().iter_mut()), + _ => None, + } + } +} + +/// Targets of an indirect call. +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)] +pub struct IndirectCallTargets(Vec); +/// Targets of an indirect jump. +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)] +pub struct IndirectJumpTargets(Vec); + +impl Deref for IndirectCallTargets { + type Target = [FunctionTid]; + + fn deref(&self) -> &[FunctionTid] { + &self.0 + } +} + +impl Deref for IndirectJumpTargets { + type Target = [BlockTid]; + + fn deref(&self) -> &[BlockTid] { + &self.0 + } +} + +impl AsRef for IndirectCallTargets +where + T: ?Sized, + ::Target: AsRef, +{ + fn as_ref(&self) -> &T { + self.deref().as_ref() + } +} + +impl AsRef for IndirectJumpTargets +where + T: ?Sized, + ::Target: AsRef, +{ + fn as_ref(&self) -> &T { + self.deref().as_ref() + } +} + +// Can we use `dyn Tid` after refactoring of `Tid` type? +impl AsRef<[Tid]> for IndirectCfTargets { + fn as_ref(&self) -> &[Tid] { + match self { + IndirectCfTargets::Jump(x) => x.as_ref(), + IndirectCfTargets::Call(x) => x.as_ref(), + } + } +} + +impl AsMut> for IndirectCallTargets { + fn as_mut(&mut self) -> &mut Vec { + &mut self.0 + } +} + +impl AsMut> for IndirectJumpTargets { + fn as_mut(&mut self) -> &mut Vec { + &mut self.0 + } +} + +impl> From for IndirectCallTargets { + fn from(iter: T) -> Self { + iter.collect() + } +} + +impl> From for IndirectJumpTargets { + fn from(iter: T) -> Self { + iter.collect() + } +} + +impl FromIterator for IndirectCallTargets { + fn from_iter>(iter: T) -> Self { + Self(iter.into_iter().collect()) + } +} + +impl FromIterator for IndirectJumpTargets { + fn from_iter>(iter: T) -> Self { + Self(iter.into_iter().collect()) + } +} + +impl IntoIterator for IndirectCallTargets { + type Item = FunctionTid; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl IntoIterator for IndirectJumpTargets { + type Item = BlockTid; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } } impl Default for Blk { @@ -56,7 +212,7 @@ impl Blk { Blk { defs: vec![], jmps: vec![], - indirect_jmp_targets: vec![], + indirect_control_flow_targets: None, } } @@ -110,6 +266,110 @@ impl Blk { acc }) } + + /// Sets the possible targets of the indirect jump at the end of this block. + pub fn set_ind_jump_targets(&mut self, v: T) -> &mut Self + where + T: IntoIterator, + { + debug_assert!(matches!( + self.jmps.first().map(|j| &j.term), + Some(Jmp::BranchInd(_)) + )); + + self.indirect_control_flow_targets = Some(Box::new(IndirectCfTargets::Jump( + IndirectJumpTargets::from_iter(v), + ))); + + self + } + + /// Sets the possible targets of the indirect call at the end of this block. + pub fn set_ind_call_targets(&mut self, v: T) -> &mut Self + where + T: IntoIterator, + { + debug_assert!(matches!( + self.jmps.first().map(|j| &j.term), + Some(Jmp::CallInd { .. }) + )); + + self.indirect_control_flow_targets = Some(Box::new(IndirectCfTargets::Call( + IndirectCallTargets::from_iter(v), + ))); + + self + } + + /// Clears the possible targets of the indirect control flow transfer at the + /// end of this block. + pub fn clear_ind_control_flow_targets(&mut self) -> &mut Self { + self.indirect_control_flow_targets = None; + + self + } + + /// Returns the possible targets of the indirect control flow transfer at + /// the end of this block. + pub fn ind_control_flow_targets(&self) -> Option<&IndirectCfTargets> { + self.indirect_control_flow_targets.as_deref() + } + + /// Returns the possible targets of the indirect control flow transfer at + /// the end of this block. + pub fn ind_control_flow_targets_mut(&mut self) -> Option<&mut IndirectCfTargets> { + self.indirect_control_flow_targets.as_deref_mut() + } + + /// Returns the possible targets of the indirect jump at the end of this + /// block. + pub fn ind_jump_targets(&self) -> Option> { + self.ind_control_flow_targets() + .and_then(|x| x.iter_ind_jump_targets()) + } + + /// Returns the possible targets of the indirect jump at the end of this + /// block. + pub fn ind_jump_targets_targets_mut(&mut self) -> Option> { + self.ind_control_flow_targets_mut() + .and_then(|x| x.iter_ind_jump_targets_mut()) + } + + /// Returns the possible targets of the indirect call at the end of this + /// block. + pub fn ind_call_targets(&self) -> Option> { + self.ind_control_flow_targets() + .and_then(|x| x.iter_ind_call_targets()) + } + + /// Returns the possible targets of the indirect call at the end of this + /// block. + pub fn ind_call_targets_targets_mut( + &mut self, + ) -> Option> { + self.ind_control_flow_targets_mut() + .and_then(|x| x.iter_ind_call_targets_mut()) + } + + /// Adds the jumps to the end of the block. + pub fn add_jumps(&mut self, jmps: T) -> &mut Self + where + T: IntoIterator>, + { + self.jmps.extend(jmps); + + self + } + + /// Adds the defs to the end of the block. + pub fn add_defs(&mut self, defs: T) -> &mut Self + where + T: IntoIterator>, + { + self.defs.extend(defs); + + self + } } /// The different kinds of sinks in a CFG. @@ -173,39 +433,61 @@ impl Term { cloned_block } - /// Remove indirect jump target addresses for which no corresponding target - /// block exists. + /// Removes indirect control flow targets that do not exist. /// /// Returns an error message for each removed address. - pub fn remove_nonexisting_indirect_jump_targets( + pub fn remove_nonexisting_indirect_cf_targets( &mut self, - all_jump_targets: &HashSet, - ) -> Result<(), Vec> { - let mut logs = Vec::new(); - - self.term.indirect_jmp_targets = self - .term - .indirect_jmp_targets - .iter() - .filter_map(|target| { - if all_jump_targets.contains(target) { - Some(target.clone()) - } else { - let error_msg = format!( - "Indirect jump target at {} does not exist", - target.address() - ); - logs.push(LogMessage::new_error(error_msg).location(self.tid.clone())); - None - } - }) - .collect(); + all_jump_targets: &HashSet, + all_call_targets: &HashSet, + ) -> Vec { + let mut logs = Vec::with_capacity(0); - if logs.is_empty() { - Ok(()) - } else { - Err(logs) - } + self.term.indirect_control_flow_targets = + self.term.indirect_control_flow_targets.as_ref().map(|b| { + Box::new(match &**b { + IndirectCfTargets::Call(indirect_call_targets) => IndirectCfTargets::Call( + indirect_call_targets + .iter() + .filter_map(|target| { + if all_call_targets.contains(target) { + Some(target.clone()) + } else { + let error_msg = format!( + "Indirect call target at {} does not exist", + target.address() + ); + logs.push( + LogMessage::new_error(error_msg).location(self.tid.clone()), + ); + None + } + }) + .collect(), + ), + IndirectCfTargets::Jump(indirect_jump_targets) => IndirectCfTargets::Jump( + indirect_jump_targets + .iter() + .filter_map(|target| { + if all_jump_targets.contains(target) { + Some(target.clone()) + } else { + let error_msg = format!( + "Indirect jump target at {} does not exist", + target.address() + ); + logs.push( + LogMessage::new_error(error_msg).location(self.tid.clone()), + ); + None + } + }) + .collect(), + ), + }) + }); + + logs } /// Returns a new artificial sink block with the given suffix attached to @@ -228,7 +510,7 @@ impl Term { term: Blk { defs: Vec::with_capacity(0), jmps, - indirect_jmp_targets: Vec::with_capacity(0), + indirect_control_flow_targets: None, }, } } @@ -250,7 +532,7 @@ impl Term { term: Blk { defs: Vec::with_capacity(0), jmps, - indirect_jmp_targets: Vec::with_capacity(0), + indirect_control_flow_targets: None, }, } } diff --git a/src/cwe_checker_lib/src/intermediate_representation/term.rs b/src/cwe_checker_lib/src/intermediate_representation/term.rs index 6cfd54279..9901a1206 100644 --- a/src/cwe_checker_lib/src/intermediate_representation/term.rs +++ b/src/cwe_checker_lib/src/intermediate_representation/term.rs @@ -22,6 +22,16 @@ pub struct Tid { /// The address where the term is located. address: TidAddress, } +// TODO: Eventually we want those to be proper Newtypes. Common behavior should +// be moved to a `Tid` trait. +/// Identifier of a program term. +pub type ProgramTid = Tid; +/// Identifier of a function term. +pub type FunctionTid = Tid; +/// Identifier of a block term. +pub type BlockTid = Tid; +/// Identifier of an instruction term. +pub type InstructionTid = Tid; /// The memory address of a term. /// @@ -309,6 +319,12 @@ pub struct Term { pub term: T, } +impl Display for Term { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}: {}", self.tid, self.term) + } +} + impl Deref for Term { type Target = T; diff --git a/src/cwe_checker_lib/src/intermediate_representation/term/builder_low_lvl.rs b/src/cwe_checker_lib/src/intermediate_representation/term/builder_low_lvl.rs index 2c5f160f6..a8daf62ad 100644 --- a/src/cwe_checker_lib/src/intermediate_representation/term/builder_low_lvl.rs +++ b/src/cwe_checker_lib/src/intermediate_representation/term/builder_low_lvl.rs @@ -194,11 +194,7 @@ impl Blk { pub fn mock_with_tid(tid: &str) -> Term { Term { tid: Tid::new(tid), - term: Blk { - defs: Vec::new(), - jmps: Vec::new(), - indirect_jmp_targets: Vec::new(), - }, + term: Blk::new(), } }