Skip to content

Commit 441926e

Browse files
danceratopzmarioevz
authored andcommitted
feat(security): implement LiteralString for subprocess security
- Add `safe_docker_exec` wrapper for Docker command construction - Implement `safe_solc_command` for compiler argument validation - Add `safe_t8n_args` for transition tool parameter security - Use `LiteralString` type hints to prevent command injection - Validate file paths, EVM versions, and numeric parameters
1 parent 859e126 commit 441926e

File tree

3 files changed

+121
-33
lines changed

3 files changed

+121
-33
lines changed

src/cli/extract_config.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import subprocess
1212
import sys
1313
from pathlib import Path
14-
from typing import Dict, Optional, Tuple, cast
14+
from typing import Dict, LiteralString, Optional, Tuple, cast
1515

1616
import click
1717
from hive.simulation import Simulation
@@ -26,9 +26,21 @@
2626
from pytest_plugins.consume.simulators.helpers.ruleset import ruleset
2727

2828

29+
def safe_docker_exec(container_id: str, command: LiteralString, *args: LiteralString) -> list[str]:
30+
"""Safely construct docker exec commands with literal strings."""
31+
return ["docker", "exec", container_id, command] + list(args)
32+
33+
34+
def safe_docker_command(command: LiteralString, *args: LiteralString) -> list[str]:
35+
"""Safely construct docker commands with literal strings."""
36+
return ["docker", command] + list(args)
37+
38+
2939
def get_docker_containers() -> set[str]:
3040
"""Get the current list of Docker container IDs."""
31-
result = subprocess.run(["docker", "ps", "-q"], capture_output=True, text=True, check=True)
41+
result = subprocess.run(
42+
safe_docker_command("ps", "-q"), capture_output=True, text=True, check=True
43+
)
3244
return set(result.stdout.strip().split("\n")) if result.stdout.strip() else set()
3345

3446

@@ -56,12 +68,12 @@ def extract_client_files(
5668
try:
5769
# Use docker exec to read the file from the container
5870
# First check if file exists
59-
check_cmd = ["docker", "exec", container_id, "test", "-f", container_path]
71+
check_cmd = safe_docker_exec(container_id, "test", "-f", container_path)
6072
check_result = subprocess.run(check_cmd, capture_output=True)
6173

6274
if check_result.returncode == 0:
6375
# File exists, now read it
64-
read_cmd = ["docker", "exec", container_id, "cat", container_path]
76+
read_cmd = safe_docker_exec(container_id, "cat", container_path)
6577
result = subprocess.run(read_cmd, capture_output=True, text=True)
6678

6779
if result.returncode == 0 and result.stdout:
@@ -267,7 +279,7 @@ def extract_config(
267279
# Optionally list files in container
268280
if list_files:
269281
click.echo("\nListing files in container root:")
270-
list_cmd = ["docker", "exec", container_id, "ls", "-la", "/"]
282+
list_cmd = safe_docker_exec(container_id, "ls", "-la", "/")
271283
result = subprocess.run(list_cmd, capture_output=True, text=True)
272284
if result.returncode == 0:
273285
click.echo(result.stdout)

src/ethereum_clis/transition_tool.py

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from abc import abstractmethod
1111
from dataclasses import dataclass
1212
from pathlib import Path
13-
from typing import Any, Dict, List, Mapping, Optional, Type
13+
from typing import Any, Dict, List, LiteralString, Mapping, Optional, Type
1414
from urllib.parse import urlencode
1515

1616
from requests import Response
@@ -20,6 +20,7 @@
2020
from ethereum_test_base_types import BlobSchedule
2121
from ethereum_test_exceptions import ExceptionMapper
2222
from ethereum_test_forks import Fork
23+
from ethereum_test_forks.helpers import get_development_forks, get_forks
2324
from ethereum_test_types import Alloc, Environment, Transaction
2425

2526
from .ethereum_cli import EthereumCLI
@@ -38,6 +39,12 @@
3839
SLOW_REQUEST_TIMEOUT = 180
3940

4041

42+
def get_valid_transition_tool_names() -> set[str]:
43+
"""Get all valid transition tool names from deployed and development forks."""
44+
all_available_forks = get_forks() + get_development_forks()
45+
return {fork.transition_tool_name() for fork in all_available_forks}
46+
47+
4148
class TransitionTool(EthereumCLI):
4249
"""
4350
Transition tool abstract base class which should be inherited by all transition tool
@@ -424,6 +431,49 @@ def _evaluate_stream(
424431

425432
return output
426433

434+
def safe_t8n_args(
435+
self, fork_name: str, chain_id: int, reward: int, temp_dir=None
436+
) -> List[str]:
437+
"""Safely construct t8n arguments with validated inputs."""
438+
# Validate fork name against actual transition tool names from all available forks
439+
valid_forks = get_valid_transition_tool_names()
440+
if fork_name not in valid_forks:
441+
raise ValueError(f"Invalid fork name: {fork_name}")
442+
443+
# Validate chain ID (should be positive integer)
444+
if not isinstance(chain_id, int) or chain_id <= 0:
445+
raise ValueError(f"Invalid chain ID: {chain_id}")
446+
447+
# Validate reward (should be non-negative integer)
448+
if not isinstance(reward, int) or reward < 0:
449+
raise ValueError(f"Invalid reward: {reward}")
450+
451+
# Use literal strings for command flags
452+
input_alloc: LiteralString = "--input.alloc=stdin"
453+
input_txs: LiteralString = "--input.txs=stdin"
454+
input_env: LiteralString = "--input.env=stdin"
455+
output_result: LiteralString = "--output.result=stdout"
456+
output_alloc: LiteralString = "--output.alloc=stdout"
457+
output_body: LiteralString = "--output.body=stdout"
458+
trace_flag: LiteralString = "--trace"
459+
460+
args = [
461+
input_alloc,
462+
input_txs,
463+
input_env,
464+
output_result,
465+
output_alloc,
466+
output_body,
467+
f"--state.fork={fork_name}",
468+
f"--state.chainid={chain_id}",
469+
f"--state.reward={reward}",
470+
]
471+
472+
if self.trace and temp_dir:
473+
args.extend([trace_flag, f"--output.basedir={temp_dir.name}"])
474+
475+
return args
476+
427477
def construct_args_stream(
428478
self, t8n_data: TransitionToolData, temp_dir: tempfile.TemporaryDirectory
429479
) -> List[str]:
@@ -432,22 +482,10 @@ def construct_args_stream(
432482
if self.subcommand:
433483
command.append(self.subcommand)
434484

435-
args = command + [
436-
"--input.alloc=stdin",
437-
"--input.txs=stdin",
438-
"--input.env=stdin",
439-
"--output.result=stdout",
440-
"--output.alloc=stdout",
441-
"--output.body=stdout",
442-
f"--state.fork={t8n_data.fork_name}",
443-
f"--state.chainid={t8n_data.chain_id}",
444-
f"--state.reward={t8n_data.reward}",
445-
]
446-
447-
if self.trace:
448-
args.append("--trace")
449-
args.append(f"--output.basedir={temp_dir.name}")
450-
return args
485+
safe_args = self.safe_t8n_args(
486+
t8n_data.fork_name, t8n_data.chain_id, t8n_data.reward, temp_dir
487+
)
488+
return command + safe_args
451489

452490
def dump_debug_stream(
453491
self,

src/ethereum_test_specs/static_state/common/compile_yul.py

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,54 @@
11
"""compile yul with arguments."""
22

33
import subprocess
4+
from pathlib import Path
5+
from typing import LiteralString
6+
7+
8+
def safe_solc_command(
9+
source_file: Path | str, evm_version: str | None = None, optimize: str | None = None
10+
) -> list[str]:
11+
"""Safely construct solc command with validated inputs."""
12+
# Validate source file path
13+
source_path = Path(source_file)
14+
if not source_path.exists():
15+
raise FileNotFoundError(f"Source file not found: {source_file}")
16+
if source_path.suffix not in (".yul", ".sol"):
17+
raise ValueError(f"Invalid file extension for solc: {source_path.suffix}")
18+
19+
cmd: list[str] = ["solc"]
20+
21+
# Add EVM version if provided (validate against known versions)
22+
if evm_version:
23+
valid_versions = {
24+
"homestead",
25+
"tangerineWhistle",
26+
"spuriousDragon",
27+
"byzantium",
28+
"constantinople",
29+
"petersburg",
30+
"istanbul",
31+
"berlin",
32+
"london",
33+
"paris",
34+
"shanghai",
35+
"cancun",
36+
}
37+
if evm_version not in valid_versions:
38+
raise ValueError(f"Invalid EVM version: {evm_version}")
39+
cmd.extend(["--evm-version", evm_version])
40+
41+
# Add compilation flags (using literal strings)
42+
strict_assembly: LiteralString = "--strict-assembly"
43+
cmd.append(strict_assembly)
44+
45+
if optimize is None:
46+
optimize_flag: LiteralString = "--optimize"
47+
yul_opts: LiteralString = "--yul-optimizations=:"
48+
cmd.extend([optimize_flag, yul_opts])
49+
50+
cmd.append(str(source_path))
51+
return cmd
452

553

654
def compile_yul(source_file: str, evm_version: str | None = None, optimize: str | None = None):
@@ -19,17 +67,7 @@ def compile_yul(source_file: str, evm_version: str | None = None, optimize: str
1967
Raises_:
2068
Exception: If the solc output contains an error message.
2169
"""
22-
cmd = ["solc"]
23-
if evm_version:
24-
cmd.extend(["--evm-version", evm_version])
25-
26-
# Choose flags based on whether flag is provided
27-
if optimize is None:
28-
# When flag is not provided, include extra optimization flags
29-
cmd.extend(["--strict-assembly", "--optimize", "--yul-optimizations=:", source_file])
30-
else:
31-
# Otherwise, omit the optimization flags
32-
cmd.extend(["--strict-assembly", source_file])
70+
cmd = safe_solc_command(source_file, evm_version, optimize)
3371

3472
# Execute the solc command and capture both stdout and stderr
3573
result = subprocess.run(

0 commit comments

Comments
 (0)