Skip to content

Commit e874651

Browse files
committed
fix(cfg): link fallback block from entry node in control flow graph
The CFG builder deduplicated blocks with a `HashSet`, returning early whenever a block had already been visited. This dropped the edge from the current parent into the existing block, so when a fallback handler was first reached through the dispatcher's fall-through path, the direct `CALLDATASIZE < 4` edge from the entry node was never added. Track visited blocks in a `HashMap<String, NodeIndex>` instead, so an already-seen block's node index can be looked up and the parent edge still linked (without recursing again). This restores the missing entry -> fallback edge, e.g. for WETH (0xc02a...cc2) node 0 now links to 0xaf. Closes #627
1 parent b1fa157 commit e874651

3 files changed

Lines changed: 58 additions & 9 deletions

File tree

crates/cfg/src/core/graph.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use heimdall_vm::{
66
ext::exec::VMTrace,
77
};
88
use petgraph::{matrix_graph::NodeIndex, Graph};
9-
use std::collections::HashSet;
9+
use std::collections::HashMap;
1010

1111
/// convert a symbolic execution [`VMTrace`] into a [`Graph`] of blocks, illustrating the
1212
/// control-flow graph found by the symbolic execution engine.
@@ -15,7 +15,7 @@ pub(crate) fn build_cfg(
1515
contract_cfg: &mut Graph<String, String>,
1616
parent_node: Option<NodeIndex<u32>>,
1717
jump_taken: bool,
18-
seen_nodes: &mut HashSet<String>,
18+
seen_nodes: &mut HashMap<String, NodeIndex<u32>>,
1919
) -> Result<()> {
2020
let mut cfg_node: String = String::new();
2121
let mut parent_node = parent_node;
@@ -46,14 +46,19 @@ pub(crate) fn build_cfg(
4646
cfg_node.push_str(&format!("{}\n", &assembly));
4747
}
4848

49-
// check if this node has been seen before
50-
if seen_nodes.contains(&cfg_node) {
49+
// if this node has been seen before, we still need to link the current parent to it,
50+
// otherwise edges into already-visited blocks (e.g. a shared fallback handler) are lost.
51+
// we don't recurse again, since the block's children have already been mapped.
52+
if let Some(&node_index) = seen_nodes.get(&cfg_node) {
53+
if let Some(parent_node) = parent_node {
54+
contract_cfg.update_edge(parent_node, node_index, jump_taken.to_string());
55+
}
5156
return Ok(());
5257
}
53-
seen_nodes.insert(cfg_node.clone());
5458

5559
// add the node to the graph
56-
let node_index = contract_cfg.add_node(cfg_node);
60+
let node_index = contract_cfg.add_node(cfg_node.clone());
61+
seen_nodes.insert(cfg_node, node_index);
5762
if let Some(parent_node) = parent_node {
5863
contract_cfg.update_edge(parent_node, node_index, jump_taken.to_string());
5964
}

crates/cfg/src/core/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use alloy::primitives::Address;
44
use eyre::eyre;
55
use heimdall_common::{ether::compiler::detect_compiler, utils::strings::StringExt};
66
use heimdall_vm::core::vm::VM;
7-
use std::collections::HashSet;
7+
use std::collections::HashMap;
88

99
use petgraph::{dot::Dot, Graph};
1010
use std::time::{Duration, Instant};
@@ -104,7 +104,7 @@ pub async fn cfg(args: CfgArgs) -> Result<CfgResult, Error> {
104104
let start_cfg_time = Instant::now();
105105
info!("building cfg for '{}' from symbolic execution trace", args.target.truncate(64));
106106
let mut contract_cfg = Graph::new();
107-
let mut seen_nodes: HashSet<String> = HashSet::new();
107+
let mut seen_nodes = HashMap::new();
108108
build_cfg(&map, &mut contract_cfg, None, false, &mut seen_nodes)?;
109109
debug!("building cfg took {:?}", start_cfg_time.elapsed());
110110

crates/core/tests/test_cfg.rs

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ mod integration_tests {
66
use std::path::PathBuf;
77

88
use heimdall_cfg::{cfg, CfgArgs, CfgArgsBuilder, HardFork};
9-
use petgraph::dot::Dot;
9+
use petgraph::{dot::Dot, graph::NodeIndex, visit::EdgeRef};
1010
use serde_json::Value;
1111

1212
#[tokio::test]
@@ -68,6 +68,50 @@ mod integration_tests {
6868
}
6969
}
7070

71+
#[tokio::test]
72+
async fn test_cfg_links_fallback_block() {
73+
// regression test for https://github.com/Jon-Becker/heimdall-rs/issues/627
74+
// the entry node dispatches to the fallback handler when `CALLDATASIZE < 4`, but the
75+
// edge into that block was previously dropped when the block had already been visited
76+
// through another path (e.g. the dispatcher's fall-through).
77+
let rpc_url = std::env::var("RPC_URL").unwrap_or_else(|_| {
78+
println!("RPC_URL not set, skipping test");
79+
std::process::exit(0);
80+
});
81+
82+
// WETH (0xc02a...cc2) exhibits the missing fallback edge from the entry node.
83+
let result = heimdall_cfg::cfg(CfgArgs {
84+
target: String::from("0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2"),
85+
rpc_url,
86+
default: true,
87+
color_edges: false,
88+
output: String::from(""),
89+
name: String::from(""),
90+
timeout: 10000,
91+
hardfork: HardFork::Latest,
92+
etherscan_api_key: String::from(""),
93+
})
94+
.await
95+
.expect("failed to generate cfg");
96+
97+
// the entry node is always the first node added to the graph.
98+
let entry = NodeIndex::new(0);
99+
assert!(
100+
result.graph[entry].contains("CALLDATASIZE"),
101+
"entry node should contain the calldatasize dispatch check"
102+
);
103+
104+
// the entry node must link to the fallback handler at 0xaf.
105+
let links_to_fallback = result
106+
.graph
107+
.edges(entry)
108+
.any(|edge| result.graph[edge.target()].starts_with("0xaf JUMPDEST"));
109+
assert!(
110+
links_to_fallback,
111+
"entry node should link to the fallback block (0xaf) when calldatasize < 4"
112+
);
113+
}
114+
71115
#[tokio::test]
72116
async fn test_cfg_auto_hardfork() {
73117
let rpc_url = std::env::var("RPC_URL").unwrap_or_else(|_| {

0 commit comments

Comments
 (0)