Back to skill

Security audit

Defi Trading Engine

Security checks for vulnerabilities and agentic risk

Overview

This skill is a real-crypto trading helper with under-scoped safety controls that could cause unintended live transactions.

Install only after reviewing the scripts and using a dedicated low-balance wallet. Do not rely on trading-config.json dry_run; pass --dry-run explicitly until the executor is fixed, never use --skip-risk-check for live trades, and pin/review the Bankr CLI before funding any wallet.

Vulnerability Patterns
  • Insecure DependenciesIntroduces malicious components through unsafe dependency sources
  • Insecure Skill Coding PracticesFinds exploitable flaws such as hardcoded secrets or command injection
  • Skill Instruction HijackingAlters the agent's session goals or safety constraints when the skill loads
  • Agent Memory PoisoningWrites attacker-controlled rules into memory that affect later sessions
  • Remote Payload Retrieval and ExecutionFetches external code whose behavior can change after review
Findings (5)

T09 · Insecure Skill Coding Practices

Error
Location
scripts/trade-executor.py:328
Finding

Live Trades Can Explicitly Bypass All Risk Controls

Content
View full analysis

Vulnerability Details

File Location: scripts/trade-executor.py, lines 328-332 and 398-411
Vulnerability Type: User-accessible risk-control bypass
Risk Level: High

Vulnerable Code

python
# 1. Risk check (unless explicitly skipped)
if not skip_risk_check and not dry_run:
    print("\n🛡️  Running risk check...")
    if not run_risk_check(symbol, amount, config_path, trades_dir):
        print("\n❌ Trade rejected by risk manager")
        return False
python
parser.add_argument('--skip-risk-check', action='store_true',
                    help='Skip risk manager check (dangerous!)')

args = parser.parse_args()

# Load config
config = load_config(args.config)

# Execute trade
success = execute_trade(
    symbol=args.symbol,
    action=args.action,
    amount=args.amount,
    config=config,
    dry_run=args.dry_run,
    skip_risk_check=args.skip_risk_check,
    config_path=args.config,
    trades_dir=args.trades_dir
)

Technical Analysis

The publicly exposed --skip-risk-check command-line option bypasses the risk manager before a live transaction. This disables all checks implemented by the risk manager, including maximum position size, active-position limits, daily trade limits, cooldown enforcement, maximum drawdown, and correlated-position limits.

This behavior directly contradicts the executor's documented claim that it always performs a risk check before executing a trade. The bypass does not require authentication, separate authorization, interactive confirmation, or an auditable emergency-override procedure.

Attack Path

  1. An attacker, compromised automation process, or operator with access to invoke the script selects a token, action, and amount.
  2. The caller adds --skip-risk-check to the command:
    bash
    python3 scripts/trade-executor.py \
      --symbol TOKEN \
      --action buy \
      --amount 5000 \
      --skip-ri
    

...[truncated 949 chars]

Remediation
View remediation

Remediation Suggestions

  • Remove --skip-risk-check from the production command-line interface.
  • Make the risk check mandatory for every live transaction.
  • If an emergency override is operationally necessary, place it in a separate administrative workflow requiring:
    • Explicit authenticated authorization.
    • Interactive confirmation displaying the complete transaction.
    • A short-lived approval token.
    • Immutable audit logging.
    • A hard-coded maximum emergency transaction size.
  • Ensure the executor fails closed if the risk-manager script is unavailable, times out, returns malformed output, or cannot load configuration.
  • Add integration tests proving that no live execution path can reach execute_bankr_swap() without successful risk approval.

T09 · Insecure Skill Coding Practices

Error
Location
scripts/trade-executor.py:396
Finding

Configured Safe Dry-Run Default Is Ignored During Execution

Content
View full analysis

Vulnerability Details

File Location: scripts/trade-executor.py, lines 17-23 and 396-411; assets/trading-config.example.json, line 12
Vulnerability Type: Fail-open live financial execution
Risk Level: Critical

Vulnerable Code

python
DEFAULTS = {
    "chain": "base",
    "trade_size_usd": 40,
    "take_profit_pct": 4.0,
    "stop_loss_pct": 8.0,
    "dry_run": True,
}
python
parser.add_argument('--dry-run', action='store_true',
                    help='Simulate trade without executing')

args = parser.parse_args()

# Load config
config = load_config(args.config)

# Execute trade
success = execute_trade(
    symbol=args.symbol,
    action=args.action,
    amount=args.amount,
    config=config,
    dry_run=args.dry_run,
    skip_risk_check=args.skip_risk_check,
    config_path=args.config,
    trades_dir=args.trades_dir
)

The example configuration also declares:

json
"dry_run": true,

Technical Analysis

The application advertises a safe default of dry_run: true, and the example configuration reinforces that expectation. However, the execution path ignores config["dry_run"] and passes only args.dry_run into execute_trade().

The store_true argument defaults to false when --dry-run is absent. Consequently, a user who sets "dry_run": true in the configuration—or relies on the missing-configuration fallback that claims to use safe defaults—can still trigger a live Bankr transaction.

The control is therefore misleading and fail-open: omission of a command-line flag enables live execution rather than requiring explicit authorization.

Attack Path

  1. The user copies the example configuration containing "dry_run": true, or runs without a configuration and relies on the documented safe default.
  2. The user invokes the documented command without the optional --dry-run flag:
    bash
    python3 scripts/trade-ex
    

...[truncated 808 chars]

Remediation
View remediation

Remediation Suggestions

  • Make simulation the unconditional default and require an explicit --live option for real execution.
  • Resolve execution mode using configuration and CLI data without allowing omission to enable live trading. For example:
    python
    live_requested = args.live
    configured_dry_run = config.get("dry_run", True)
    dry_run = configured_dry_run or not live_requested
    
  • Reject contradictory settings instead of silently selecting the less safe behavior.
  • Fail closed when configuration is missing or invalid.
  • Before live execution, display the token, action, amount, chain, wallet identity, and slippage and require explicit confirmation unless a separately authorized automation mode is configured.
  • Add tests covering missing configuration, "dry_run": true, omitted flags, and explicit live execution.

T09 · Insecure Skill Coding Practices

Error
Location
scripts/risk-manager.py:173
Finding

Negative and Non-Finite Trade Amounts Pass Risk Validation

Content
View full analysis

Vulnerability Details

File Location: scripts/risk-manager.py, lines 173-175 and 315-316; scripts/trade-executor.py, lines 393-394
Vulnerability Type: Improper numeric input validation
Risk Level: High

Vulnerable Code

python
# 1. Position size check
max_position = risk.get('max_position_size_usd', 40)
if amount > max_position:
    return False, f"Position size ${amount} exceeds limit ${max_position}"
python
parser.add_argument('--amount', type=float, help='Trade amount in USD')

The executor similarly accepts an unrestricted float:

python
parser.add_argument('--amount', type=float, required=True,
                    help='Trade amount in USD')

Technical Analysis

The risk manager only checks whether the amount is greater than the configured maximum. It does not require a positive value and does not verify that the number is finite.

Python's float parser accepts values such as nan, inf, and -inf. In particular, comparisons involving NaN are false, so amount > max_position does not reject NaN. Zero, negative values, and negative infinity also remain below the maximum and pass this check.

Once approved, the amount is forwarded to Bankr as a command argument and is used in quantity, position, log, and drawdown calculations. The external CLI's handling of malformed or signed values is not controlled by this project.

Attack Path

  1. A caller supplies a malformed amount:
    bash
    python3 scripts/trade-executor.py \
      --symbol SOL \
      --action buy \
      --amount nan
    
    Alternatively, the caller supplies a negative value such as --amount -40.
  2. argparse converts the string to a Python float.
  3. The maximum-position comparison does not reject the value.
  4. The risk manager approves the request if unrelated controls pass.
  5. The executor forwards nan or the negative amount to Bankr and uses it in local calc ...[truncated 675 chars]
Remediation
View remediation

Remediation Suggestions

  • Validate amounts before any risk calculation or subprocess invocation:
    python
    import math
    
    if not math.isfinite(amount) or amount <= 0:
        return False, "Trade amount must be a positive finite number"
    if not math.isfinite(max_position) or max_position <= 0:
        return False, "Invalid maximum-position configuration"
    if amount > max_position:
        return False, "Position size exceeds configured limit"
    
  • Parse financial values with decimal.Decimal rather than binary floating-point where practical.
  • Apply explicit lower and upper bounds to every amount, price, percentage, slippage, and quantity.
  • Validate numeric configuration values at startup and terminate on malformed settings.
  • Revalidate the amount in both the executor and risk manager to preserve defense in depth.
  • Add tests for zero, negative numbers, NaN, positive and negative infinity, extremely large values, and excessive decimal precision.

T09 · Insecure Skill Coding Practices

Error
Location
scripts/trade-executor.py:345
Finding

Protective Order Failures Are Ignored After Successful Purchases

Content
View full analysis

Vulnerability Details

File Location: scripts/trade-executor.py, lines 345-378
Vulnerability Type: Failure to enforce post-trade safety controls
Risk Level: High

Vulnerable Code

python
# 4. Set stop-loss and take-profit (for buy orders)
risk = config.get('risk', {})
stop_loss_price = None
take_profit_price = None

if action.lower() in ['buy', 'limit_buy'] and current_price:
    stop_loss_pct = risk.get('stop_loss_pct', 8)
    take_profit_pct = risk.get('take_profit_pct', 4)

    stop_loss_price = current_price * (1 - stop_loss_pct / 100)
    take_profit_price = current_price * (1 + take_profit_pct / 100)

    print(f"\n🛡️  Setting stop-loss at ${stop_loss_price:.2f} (-{stop_loss_pct}%)")
    set_stop_loss(symbol, stop_loss_price, config, dry_run)

    print(f"🎯 Setting take-profit at ${take_profit_price:.2f} (+{take_profit_pct}%)")
    set_take_profit(symbol, take_profit_price, config, dry_run)

# 5. Log trade
trade_data = {
    'timestamp': datetime.now().isoformat() + 'Z',
    'symbol': symbol,
    'action': action,
    'amount_usd': amount,
    'price': current_price,
    'quantity': quantity,
    'tx_hash': tx_hash,
    'status': 'success' if success else 'failed',
    'stop_loss_price': stop_loss_price,
    'take_profit_price': take_profit_price,
    'details': details
}

Technical Analysis

Both set_stop_loss() and set_take_profit() return Boolean success indicators, but the caller ignores those values. The workflow marks the trade as successful based solely on the initial swap result.

Therefore, a buy can complete while one or both protective orders fail because of a timeout, unsupported CLI syntax, network failure, insufficient permissions, or an exchange-side rejection. Despite this failure, the script logs stop-loss and take-profit prices and prints Trade complete, creating a false indication that the position is protected.

Attack Pa

...[truncated 1148 chars]

Remediation
View remediation

Remediation Suggestions

  • Capture and verify both return values:
    python
    stop_ok = set_stop_loss(symbol, stop_loss_price, config, dry_run)
    take_profit_ok = set_take_profit(symbol, take_profit_price, config, dry_run)
    
    if not stop_ok or not take_profit_ok:
        # Mark the position as unprotected and invoke recovery handling.
        ...
    
  • Record separate states such as swap_success, stop_loss_status, take_profit_status, and protection_status.
  • Treat stop-loss failure as a high-priority incident requiring immediate alerting.
  • Implement bounded retries with idempotency protection.
  • Define a safe recovery policy, such as manually escalating the position or unwinding it when mandatory protection cannot be established.
  • Confirm protective orders through a query to Bankr rather than relying only on command exit status.
  • Never print overall success unless all mandatory workflow stages have succeeded.

T08 · Insecure Dependencies

Warning
Location
references/bankr-setup.md:6
Finding

Unpinned Bankr CLI Is Installed Globally and Used for Funded-Wallet Operations

Content
View full analysis

Vulnerability Details

File Location: references/bankr-setup.md, line 6
Vulnerability Type: Unsafe third-party dependency installation
Risk Level: Medium

Vulnerable Code

bash
npm install -g @bankr/cli

Technical Analysis

The setup documentation installs the latest available version of @bankr/cli globally without a version pin, lockfile, integrity hash, or documented publisher verification. The installed executable is subsequently invoked by the trade executor and operates in the context of an authenticated Bankr account and funded wallet.

A future compromised, malicious, or behaviorally incompatible upstream release would therefore enter the execution path without another review of this Skill. Global installation also broadens the dependency's filesystem and command-execution scope relative to a project-local isolated installation.

No evidence was found that the current package is malicious. The vulnerability is the unsafe and mutable supply-chain trust model.

Attack Path

  1. An attacker compromises the package publisher account, npm distribution channel, or a future package release.
  2. A user follows the setup instructions and runs:
    bash
    npm install -g @bankr/cli
    
  3. npm retrieves and installs the current unpinned package version and its dependency tree.
  4. Installation scripts, if present, execute with the installing user's privileges.
  5. The Skill later invokes the installed bankr executable for pricing, swaps, stop orders, and limit orders.
  6. A compromised executable can access information available to the user process, interact with Bankr credentials, falsify command output, or submit unauthorized financial operations.

Impact Assessment

The dependency executes with the operating-system privileges of the user who installs or invokes it. Its practical scope may include the global npm installation location, user-readable files, Bankr configuration s ...[truncated 332 chars]

Remediation
View remediation

Remediation Suggestions

  • Pin the dependency to a reviewed exact version, for example @bankr/cli@X.Y.Z.
  • Record and verify package integrity through a lockfile and the expected npm integrity digest.
  • Document the expected registry, package publisher, repository, and release-signing information.
  • Prefer a project-local installation over a global installation and invoke it through an explicit trusted path.
  • Run the CLI in a restricted environment with only the filesystem and network access needed for trading.
  • Review package lifecycle scripts and transitive dependencies before installation.
  • Use a dedicated low-value trading wallet with limited funds instead of a primary wallet.
  • Establish a controlled dependency-update process that includes code review, release-note review, integrity verification, and paper-trading tests before deployment.
Vulnerability Patterns
  • Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
  • Excessive AgencyUnrestricted Tool Access, Autonomous Decision Making, Scope Creep
  • Rogue AgentSelf-Modification, Session Persistence
  • Trigger AbuseOverly Broad Trigger, Shadow Command Trigger, Keyword Baiting Trigger
  • Behavioral ASTexec() Call, eval() Call, Dynamic Import
Findings (18)

Tp4

High
Category
MCP Tool Poisoning
Confidence
94% confidence
Finding

The declared description presents a full autonomous DeFi trading engine with integrations and self-improvement features. The supplied code does not execute trades, connect to blockchains, wallets, exchanges, or Bankr, and does not implement any review/self-improvement loop. Instead, it is a standalone risk gatekeeper CLI that reads local JSON config/trade files and determines whether a proposed trade should be allowed under predefined limits. This is a materially different primary purpose, so the description does not accurately represent the code chunk.

Content

No source excerpt is available for this finding.

Tp4

High
Category
MCP Tool Poisoning
Confidence
97% confidence
Finding

The description promises a broad DeFi trading engine with autonomous trading and self-improvement features. The supplied code only performs market-data retrieval from CoinGecko, heuristic scoring, and JSON output of token candidates. This is related to trading research, but it is materially narrower than the declared purpose and lacks the core advertised capabilities such as executing strategies, interacting with DeFi/Base chain infrastructure, or integrating with Bankr. Therefore the description does not accurately represent the actual behavior.

Content

No source excerpt is available for this finding.

Tp4

High
Category
MCP Tool Poisoning
Confidence
95% confidence
Finding

The code does relate to DeFi/crypto trading and Bankr/Base usage, so the domain is broadly aligned. However, the declared description materially overstates the functionality. The script is a straightforward trade executor: it accepts CLI arguments, optionally invokes an external risk-manager script, calls the Bankr CLI to swap assets and place stop-loss/take-profit orders, and logs results. There is no evidence of autonomous operation, strategy generation, self-improvement, review loops, or OpenClaw-agent-specific behavior. Because these are central aspects of the declared purpose rather than minor marketing language, this is a meaningful description-behavior mismatch.

Content

No source excerpt is available for this finding.

Undeclared Tool Scope

Medium
Category
MCP Least Privilege
Confidence
90% confidence
Finding

The skill instructs the agent to perform shell commands, copy files, read/write local data, and use networked tooling, but it does not declare any explicit tool scope or permissions boundary. In an agent environment, this increases the chance of overbroad execution and makes it harder for operators to understand or constrain filesystem, shell, and network effects before use.

Content

No source excerpt is available for this finding.

Missing User Warnings

Medium
Category
Not specified by scanner
Confidence
92% confidence
Finding

The skill presents setup and automation flows for scanning and executing trades, including cron-based operation, without placing a prominent real-funds warning in the primary usage path. In the context of DeFi trading and wallet-linked execution, this can lead users to trigger irreversible financial transactions or unattended trading without adequate confirmation, paper-trading defaults, or operational safeguards.

Content

No source excerpt is available for this finding.

Session Persistence

Medium
Category
Rogue Agent
Confidence
88% confidence
Finding

The skill directs the user to create a persistent workspace in the home directory and store ongoing trade logs, reviews, and copied scripts there. In an agent setting, persistent local state can accumulate sensitive financial activity, strategy data, and potentially wallet-related artifacts across sessions, increasing exposure if the environment is shared, compromised, or later reused unintentionally.

Content

Scanner excerpt · SKILL.md (reported line 40)May include surrounding context.

1. Setup

Create workspace:

bash
mkdir -p ~/trading-bot/{trades,reviews}

Missing User Warnings

Medium
Category
Not specified by scanner
Confidence
94% confidence
Finding

The guide lists live Bankr trading commands such as swaps, limit orders, stop-losses, and DCA/TWAP execution without a prominent warning that these commands can place real on-chain trades and cause irreversible financial loss. In the context of an autonomous DeFi trading skill, users or agents may treat these examples as safe operational guidance and execute them against funded wallets, increasing the chance of unintended trades or losses.

Content

No source excerpt is available for this finding.

Missing User Warnings

Medium
Category
Not specified by scanner
Confidence
95% confidence
Finding

The file provides concrete live-trading strategy rules, allocations, position sizing, and execution parameters without an explicit warning that following them can result in real financial loss. In the context of an autonomous DeFi trading skill, this is more dangerous because users may treat the guidance as operationally safe and directly wire it into automated execution, increasing the chance of uninformed losses.

Content

No source excerpt is available for this finding.

External Transmission

Medium
Category
Data Exfiltration
Confidence
60% confidence
Finding

Data is being sent to an external URL. This could be legitimate telemetry or data exfiltration. Manual review is recommended.

Content

Scanner excerpt · scripts/scan-tokens.py (reported line 56)May include surrounding context.

python
def fetch_trending_coins():
    """Fetch trending coins from CoinGecko."""
    trending = fetch_json("https://api.coingecko.com/api/v3/search/trending")
    if not trending or "coins" not in trending:
        return []

External Transmission

Medium
Category
Data Exfiltration
Confidence
60% confidence
Finding

Data is being sent to an external URL. This could be legitimate telemetry or data exfiltration. Manual review is recommended.

Content

Scanner excerpt · scripts/scan-tokens.py (reported line 68)May include surrounding context.

python
def fetch_trending_coins():
    """Fetch trending coins from CoinGecko."""
    trending = fetch_json("https://api.coingecko.com/api/v3/search/trending")
    if not trending or "coins" not in trending:
        return []

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
96% confidence
Finding

The module docstring states that the tool always runs risk checks before executing, but the actual CLI supports skipping them. In a DeFi trading skill, that mismatch can mislead operators or higher-level agents into assuming trades are safety-gated when they are not, increasing the chance of unauthorized or high-risk trades.

Content

No source excerpt is available for this finding.

subprocess module call

Medium
Category
Dangerous Code Execution
Confidence
70% confidence
Finding

subprocess module calls execute external commands. Without careful input validation, this enables command injection.

Content

Scanner excerpt · scripts/trade-executor.py (reported line 39)May include surrounding context.

python
def run_risk_check(symbol: str, amount: float, config_path: str, trades_dir: str) -> bool:
    """Run risk manager check before executing trade"""
    try:
        result = subprocess.run(
            [
                'python3', 'risk-manager.py',
                '--action', 'check',

subprocess module call

Medium
Category
Dangerous Code Execution
Confidence
70% confidence
Finding

subprocess module calls execute external commands. Without careful input validation, this enables command injection.

Content

Scanner excerpt · scripts/trade-executor.py (reported line 74)May include surrounding context.

python
# Attempt to get from bankr balance (if we hold it)
    try:
        result = subprocess.run(
            ['bankr', 'price', symbol],
            capture_output=True,
            text=True,

subprocess module call

Medium
Category
Dangerous Code Execution
Confidence
70% confidence
Finding

subprocess module calls execute external commands. Without careful input validation, this enables command injection.

Content

Scanner excerpt · scripts/trade-executor.py (reported line 149)May include surrounding context.

python
# Execute command
    try:
        result = subprocess.run(
            cmd,
            capture_output=True,
            text=True,

subprocess module call

Medium
Category
Dangerous Code Execution
Confidence
70% confidence
Finding

subprocess module calls execute external commands. Without careful input validation, this enables command injection.

Content

Scanner excerpt · scripts/trade-executor.py (reported line 206)May include surrounding context.

python
return True
    
    try:
        result = subprocess.run(cmd, capture_output=True, text=True, timeout=30)
        print(result.stdout)
        
        if result.returncode != 0:

subprocess module call

Medium
Category
Dangerous Code Execution
Confidence
70% confidence
Finding

subprocess module calls execute external commands. Without careful input validation, this enables command injection.

Content

Scanner excerpt · scripts/trade-executor.py (reported line 240)May include surrounding context.

python
return True
    
    try:
        result = subprocess.run(cmd, capture_output=True, text=True, timeout=30)
        print(result.stdout)
        
        if result.returncode != 0:

Missing User Warnings

Medium
Category
Not specified by scanner
Confidence
97% confidence
Finding

The --skip-risk-check flag allows live trade execution without any confirmation prompt or privileged control. In the context of an autonomous DeFi trading engine, this materially increases the likelihood of loss-causing trades, because an agent, wrapper script, or user can disable the only stated pre-trade safety control and still submit real orders.

Content

No source excerpt is available for this finding.

Natural-Language Policy Violations

Low
Category
Not specified by scanner
Confidence
91% confidence
Finding

The description explicitly limits the skill to 'OpenClaw agents' rather than offering a more generic, user-selectable language or locale. This is a natural-language policy violation because it imposes a specific language/locale constraint without user opt-in.

Content

No source excerpt is available for this finding.

Static analysis

No suspicious patterns detected.