fix: security audit fixes for TokenStandardIntrospection.sol

[rroland10]

Security Audit Report: TokenStandardIntrospection.sol

Summary

Comprehensive security audit of contracts/dex-core/TokenStandardIntrospection.sol identifying 5 vulnerabilities (2 HIGH, 1 MEDIUM, 1 LOW, 1 INFORMATIONAL) with fixes applied and verified to compile cleanly with solc 0.7.6.

Contract Purpose

TokenStandardIntrospection is a utility contract used by the Dex223 Factory to determine whether a given token address supports the ERC-223 standard. It implements the tokenReceived(address,uint256,bytes) hook: when an ERC-223 token is transferred to this contract, the token contract calls tokenReceived, and the contract records the deposit. A subsequent call to depositedAmount(_token) reveals whether the transfer was processed (i.e., the token supports ERC-223).

---

Vulnerabilities Found & Fixes Applied

V-TSI-01 — Deposit Overwrite Instead of Accumulation (HIGH)

Location: TokenStandardIntrospection.sol:9

Description: The original code uses assignment (=) instead of addition (+=):

erc223Deposits[msg.sender] = _value;  // VULNERABLE: overwrites previous deposits

If the same ERC-223 token sends multiple transfers to this contract, each new deposit completely overwrites the previous recorded balance. This causes the introspection data to misrepresent the actual cumulative deposits. While this contract is primarily used as a one-shot probe, using = is semantically incorrect and can cause silent data loss if the contract is re-used or if multiple probes occur.

Impact: Data integrity — previously recorded deposit amounts are silently erased by subsequent transfers from the same token.

Fix: Changed = to += so deposits accumulate correctly:

erc223Deposits[msg.sender] += _value;  // FIXED: accumulates correctly

---

V-TSI-02 — No Access Control on tokenReceived — Spoofable Deposits (HIGH)

Location: TokenStandardIntrospection.sol:7-11

Description: The tokenReceived function has no access control. Anyone can call it directly:

// Attacker calls directly without any actual token transfer:
introspection.tokenReceived(attacker, 999999, "");
// Now erc223Deposits[attacker_address] == 999999

This sets erc223Deposits[caller_address] to an arbitrary value, making depositedAmount() return fraudulent data. An attacker could make any arbitrary address appear to be an ERC-223 token that deposited funds.

In the ERC-223 standard, tokenReceived is called by the token contract itself during transfer(), so msg.sender should always be a contract. There is no on-chain way to verify that msg.sender is truly an ERC-223 token without an external registry, but we can at minimum verify the caller is a contract (not an EOA).

Impact: Any EOA can forge deposit records, corrupting the introspection data that the Factory relies on.

Fix: Added an extcodesize check to ensure the caller is a contract:

uint256 codeSize;
assembly { codeSize := extcodesize(caller()) }
require(codeSize > 0, "TSI: caller must be a contract");

Residual risk: A malicious contract could still call tokenReceived without an actual ERC-223 transfer. This is acceptable because the contract's purpose is to probe token standard support, not to hold funds. The extcodesize check eliminates the much more likely EOA spoofing vector.

---

V-TSI-03 — No Zero-Value Check Allows Deposit Record Erasure (MEDIUM)

Location: TokenStandardIntrospection.sol:9

Description: The original code allows _value == 0 to be recorded. Combined with vulnerability V-TSI-01 (the = assignment), a zero-value call would overwrite a real deposit with 0, effectively erasing the introspection record. Even with +=, a zero-value transfer is meaningless and wastes gas.

Impact: Deposit records could be erased (with the = bug), or gas wasted (with +=).

Fix: Added zero-value rejection:

require(_value > 0, "TSI: zero value");

---

V-TSI-04 — Missing Events for Off-Chain Monitoring (LOW)

Location: Entire contract

Description: The original contract emits no events, making it impossible for off-chain indexers, monitoring dashboards, or security systems to track deposit activity. Events are critical for transparency and auditability in DeFi protocols.

Impact: No off-chain visibility into deposit activity on this contract.

Fix: Added ERC223DepositRecorded event emitted on each successful deposit:

event ERC223DepositRecorded(address indexed token, address indexed from, uint256 value);
// ...
emit ERC223DepositRecorded(msg.sender, _from, _value);

---

V-TSI-05 — Magic Number for Return Value Not Documented (INFORMATIONAL)

Location: TokenStandardIntrospection.sol:10

Description: The return value 0x8943ec02 is used as a magic number without any documentation. This is actually bytes4(keccak256("tokenReceived(address,uint256,bytes)")), the standard ERC-223 return selector. Without documentation, future developers may not understand its purpose or may accidentally change it.

Fix: Replaced with a named constant:

bytes4 private constant _ERC223_RECEIVED = 0x8943ec02;
// ...
return _ERC223_RECEIVED;

---

Compilation Verification

• Verified compilation with solc 0.7.6 (the project's configured compiler version) — SUCCESS
• Only warning: unused _data parameter (required by ERC-223 interface signature, cannot be removed)
• The pre-existing hardhat compile failure (Revenue_old.sol and RevenueV1.sol using ^0.8.13 without a matching compiler config) is not related to these changes

Interface Compatibility

The ITokenStandardIntrospection interface (contracts/interfaces/ITokenStandardIntrospection.sol) remains fully compatible — no changes needed. Events and internal constants don't appear in interfaces.

---

Test Plan

• Deploy TokenStandardIntrospection on a local Hardhat network
• Verify that calling tokenReceived from an EOA reverts with "TSI: caller must be a contract" (V-TSI-02 fix)
• Verify that calling tokenReceived with _value == 0 from a contract reverts with "TSI: zero value" (V-TSI-03 fix)
• Deploy a mock ERC-223 token; transfer tokens to TokenStandardIntrospection; verify depositedAmount returns the correct cumulative amount after multiple transfers (V-TSI-01 fix)
• Verify ERC223DepositRecorded event is emitted with correct token, from, and value fields (V-TSI-04 fix)
• Verify the return value from tokenReceived matches 0x8943ec02 (V-TSI-05 — named constant)
• Verify Dex223Factory can still use ITokenStandardIntrospection interface without any ABI mismatch
• Verify no new compilation errors are introduced (only pre-existing Revenue file pragma mismatch)