diff --git a/.github/workflows/security-review.yml b/.github/workflows/security-review.yml index a454983..9d3abed 100644 --- a/.github/workflows/security-review.yml +++ b/.github/workflows/security-review.yml @@ -33,10 +33,10 @@ jobs: - name: Clone Security Agent run: | git clone https://github.com/security-ai-labs/security-ai-agent.git temp-agent + cp -r temp-agent/src . + cp -r temp-agent/config . cp temp-agent/main.py . - cp temp-agent/web3_analyzer.py . cp temp-agent/github_pr_commenter.py . - cp temp-agent/security_rules.py . cp temp-agent/requirements.txt . rm -rf temp-agent @@ -50,8 +50,4 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} PR_NUMBER: ${{ github.event.pull_request.number }} - REPO_NAME: ${{ github.repository }} - - - name: Analysis Complete - if: always() - run: echo "✅ Security analysis completed" \ No newline at end of file + REPO_NAME: ${{ github.repository }} \ No newline at end of file diff --git a/README.md b/README.md index 3e2d1d1..bff2c90 100644 --- a/README.md +++ b/README.md @@ -7,3 +7,5 @@ This repository contains intentionally vulnerable code samples for testing the * ## Repository Structure # Test PR + +# Test PR diff --git a/ethereum/vulnerable_erc20.sol b/ethereum/vulnerable_erc20.sol index d2620f6..06da8d4 100644 --- a/ethereum/vulnerable_erc20.sol +++ b/ethereum/vulnerable_erc20.sol @@ -1,103 +1,60 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.7.0; // ❌ CRITICAL: Old Solidity version - Integer Overflow Risk +pragma solidity ^0.7.6; -/** - * VulnerableERC20 Token - * - * Intentionally vulnerable ERC20 implementation for security testing - * DO NOT USE IN PRODUCTION - * - * Vulnerabilities included: - * - Integer Overflow/Underflow (Solidity < 0.8) - * - Missing Access Control - * - Reentrancy Vulnerability - * - Missing Zero Address Check - * - Timestamp Dependency - */ - -contract VulnerableERC20 { - - string public name = "Vulnerable Token"; - string public symbol = "VULN"; +contract RandomToken { + string public name = "RandomToken"; + string public symbol = "RND"; uint8 public decimals = 18; - uint256 public totalSupply = 1000000 * (10 ** uint256(decimals)); - - // ❌ CRITICAL: Hardcoded admin key - address constant ADMIN = 0x1234567890123456789012345678901234567890; - - mapping(address => uint256) public balances; + + uint256 public totalSupply; + address public owner; + + mapping(address => uint256) public balanceOf; mapping(address => mapping(address => uint256)) public allowance; - - event Transfer(address indexed from, address indexed to, uint256 value); - event Approval(address indexed owner, address indexed spender, uint256 value); - - constructor() { - balances[msg.sender] = totalSupply; - } - - // ❌ CRITICAL: No access control on mint - // ❌ INTEGER OVERFLOW: No SafeMath in Solidity 0.7 - function mint(address to, uint256 amount) public { - totalSupply += amount; // ❌ Can overflow - balances[to] += amount; // ❌ Can overflow - emit Transfer(address(0), to, amount); + + constructor(uint256 _initialSupply) { + owner = msg.sender; + totalSupply = _initialSupply; + balanceOf[msg.sender] = _initialSupply; } - - // ❌ CRITICAL: Reentrancy vulnerability - function withdraw(uint256 amount) public { - require(balances[msg.sender] >= amount, "Insufficient balance"); - - // ❌ REENTRANCY: Call before state update - (bool success, ) = msg.sender.call{value: amount}(""); - require(success, "Transfer failed"); - - // State update AFTER call - vulnerable! - balances[msg.sender] -= amount; + + function transfer(address to, uint256 value) external returns (bool) { + balanceOf[msg.sender] -= value; + balanceOf[to] += value; + + if (isContract(to)) { + (bool success, ) = to.call( + abi.encodeWithSignature("tokenFallback(address,uint256)", msg.sender, value) + ); + success; + } + + return true; } - - // ❌ HIGH: Unchecked call return value - function unsafeWithdraw(uint256 amount) public { - require(balances[msg.sender] >= amount); - msg.sender.call{value: amount}(""); // ❌ No require check - balances[msg.sender] -= amount; + + function approve(address spender, uint256 value) external returns (bool) { + allowance[msg.sender][spender] = value; + return true; } - - // ❌ MEDIUM: Missing zero address check - function transfer(address to, uint256 amount) public { - require(balances[msg.sender] >= amount, "Insufficient balance"); - - // ❌ No check if 'to' is address(0) - balances[msg.sender] -= amount; - balances[to] += amount; - emit Transfer(msg.sender, to, amount); + + function transferFrom(address from, address to, uint256 value) external returns (bool) { + allowance[from][msg.sender] -= value; + balanceOf[from] -= value; + balanceOf[to] += value; + return true; } - - // ❌ HIGH: No access control - anyone can burn - function burn(address account, uint256 amount) public { - // ❌ No onlyOwner check - balances[account] -= amount; - totalSupply -= amount; - emit Transfer(account, address(0), amount); + + function mint(address to, uint256 value) external { + require(tx.origin == owner, "not authorized"); + totalSupply += value; + balanceOf[to] += value; } - - // ❌ MEDIUM: Timestamp dependency - function claimDailyBonus() public { - // ❌ Miners can manipulate block.timestamp - if (block.timestamp % 86400 == 0) { - balances[msg.sender] += 1000; + + function isContract(address account) internal view returns (bool) { + uint256 size; + assembly { + size := extcodesize(account) } + return size > 0; } - function approve(address spender, uint256 amount) public { - allowance[msg.sender][spender] = amount; - emit Approval(msg.sender, spender, amount); - } - - function balanceOf(address account) public view returns (uint256) { - return balances[account]; - } - - function balanceOf2(address account) public view returns (uint256) { - return balances[account]; - } -} \ No newline at end of file +}