diff --git a/README.md b/README.md index aa693c8..42609d2 100644 --- a/README.md +++ b/README.md @@ -1,12 +1,8 @@ -// TODO: Add and explain all the attack vectors exists on this planet. - ## List of Security Vulnerabilities > Author: Harendra Shakya ([LinkTree](https://linktr.ee/harendra_shakya)) -- Reentrancy -- Oracle Manipulation -- Access Control +- [Access Control](attack-vectors/Access_Control.md) - Authentication With tx.origin - Default Visibility - Signature Verification @@ -15,96 +11,81 @@ - Missed Modifier - Incorrect Modifier Names - Overpowered Roles -- Signature Replay -- Unsafe Delegatecalls -- Sandwich Attacks -- Flash Loan Attacks -- Griefing -- Force Feeding -- Account Existence Check for low level calls -- Cross-Chain Bridge manipulation -- Insecure Randomness -- Accessing Private Data -- Authentication With tx.origin -- Delegatecall -- Integer Arithmetic -- Block timestamp Manipulation -- Unsafe external calls -- Unchecked return values -- Proxy Storage Collision -- Floating Point Arithmetic -- Code Injection via delegatecall -- Unchecked External Calls -- Insufficient Gas Attacks -- DOS +- [Account Existence Check for low level calls](attack-vectors/Account_Existence_Check_for_low_level_calls.md) +- [Arbitrary Jumps with Function Variables](attack-vectors/Arbitrary_Jumps_with_Function_Variables.md) +- [Assert Violation](attack-vectors/Assert_Violation.md) +- [Bypass Contract Size Check](attack-vectors/Bypass_Contract_Size_Check.md) +- [Code With No Effects](attack-vectors/Code_With_No_Effects.md) +- [Complex Modifiers](attack-vectors/Complex_Modifiers.md) +- [DOS](attack-vectors/DOS.md) - Unexpected Revert - Block Gas Limit - External Calls without Gas Stipends -- Offline Owner -- Entropy Illusion -- Privacy Illusion -- Miner Attacks - - Transaction Ordering / Frontrunning - - Timestamp Manipulation -- Unexpected Ether -- External Contract Referencing -- Uninitialized Storage Pointers -- Writes to Arbitrary Storage Locations -- Incorrect Interface -- Arbitrary Jumps with Function Variables -- Variable Shadowing -- Assert Violation -- Dirty Higher Order Bits -- Complex Modifiers -- Outdated Compiler -- Use of Deprecated Solidity Functions -- Function Selector Abuse -- Experimental Language Features -- Constructor call -- Frontend (Off Chain) Attacks +- [Dirty Higher Order Bits](attack-vectors/Dirty_Higher_Order_Bits.md) +- [Entropy Illusion / Insecure Randomness](attack-vectors/Entropy_Illusion.md) +- [Experimental Language Features](attack-vectors/Entropy_Illusion.md) +- [External Contract Referencing](attack-vectors/External_Contrac_Referencing.md) +- [Flash Loan Attacks](attack-vectors/Flash_Loan_Attack.md) +- [Floating Point Arithmetic](attack-vectors/Floating_Point_Arithmetic.md) +- [Frontend (Off Chain) Attacks]() - Short Address Attack -- Historic Attacks +- [Force Feeding](attack-vectors/Force_Feeding.md) +- [Function Selector Abuse](attack-vectors/Function_Selector_Abuse.md) +- [Griefing](attack-vectors/Griefing.md) +- [Hiding Malicious Code](attack-vectors/Hidden_malicious_code.md) +- [Historic Attacks](attack-vectors/Historic_Attacks.md) - Constructor Names - Call Depth Attack - Constantinople Reentrancy - Solidity Abi Encoder v2 Bug -- Payable Multicall -- Bypass Contract Size Check -- Code With No Effects -- Logical Issues -- Floating Pragma -- Hash Collisions With Multiple Variable Length Arguments -- Improper Array Deletion -- Insufficient gas griefing -- Unsafe Ownership Transfer -- Loop through long arrays -- Message call with hardcoded gas amount -- Precision Loss in Calculations -- Hiding Malicious Code with External Contract -- Public burn -- Requirement Violation -- Right-To-Left-Override control character (U+202E) -- Signature Malleability -- Transaction Order Dependence -- Typographical Error -- Unprotected Upgrades -- Unused Variable -- Wrong inheritance -- Governance Attack -- Honeypot +- [Improper Array Deletion](attack-vectors/Improper_Array_Deletion.md) +- [Incorrect Interface](attack-vectors/Incorrect_Interface.md) +- [Insufficient Gas Attacks](attack-vectors/Insufficient_Gas_Attacks.md) +- [Integer Arithmetic](attack-vectors/Integer_Arithmetic.md) +- [Loop through long arrays](attack-vectors/Loop_through_long_arrays.md) +- [Message call with hardcoded gas amount](attack-vectors/Message_call_with_hardcoded_gas_amount.md) +- [Miner Attacks](attack-vectors/Miners_Attack.md) + - Transaction Ordering / Frontrunning + - Timestamp Manipulation +- [Offline Owner](attack-vectors/Offline_Owner.md) +- [Oracle Manipulation](attack-vectors/Oracle_Manipulation.md) +- [Outdated Compiler](attack-vectors/Outdated_Compiler.md) +- [Payable Multicall](attack-vectors/Payable_Multicall.md) +- [Precision Loss in Calculations](attack-vectors/Precision_Loss_in_Calculations.md) +- [Privacy Illusion](attack-vectors/Privacy_Illusion.md) +- [Proxy Storage Collision](attack-vectors/Proxy_Storage_Collision.md) +- [Reentrancy](attack-vectors/Reentrancy.md) +- [Right-To-Left-Override control character (U+202E)]() +- [Sandwich Attacks](attack-vectors/Sandwich_Attack.md) +- [Signature Replay](attack-vectors/Signature_Replay.md) +- [Unchecked External Calls](attack-vectors/Unchecked_External_Calls.md) +- [Uninitialized Storage Pointers](attack-vectors/Uninitialized_Storage_Pointers.md) +- [Unprotected Upgrades](attack-vectors/Unprotected_Upgrades.md) +- [Unsafe Delegatecalls](attack-vectors/Unsafe_Delegatecall.md) +- [Unused Variable](attack-vectors/Unused_Variable.md) +- [Use of Deprecated Solidity Functions](attack-vectors/Use_of_Deprecated_Solidity_Functions.md) +- [Variable Shadowing](attack-vectors/Variable_Shadowing.md) +- [Writes to Arbitrary Storage Locations](attack-vectors/Writes_to_Arbitrary_Storage_Locations.md) +- [Wrong inheritance](attack-vectors/Wrong_inheritance.md) # # References -[Rekt news](https://rekt.news/) - [SWC Registry](https://swcregistry.io/) -[DeFi-Threat](https://github.com/manifoldfinance/defi-threat) +[Sigmaprime Solidity Security](https://blog.sigmaprime.io/solidity-security.html) -[Runtimeverification - List-of-Security-Vulnerabilties](https://github.com/runtimeverification/verified-smart-contracts/wiki/List-of-Security-Vulnerabilities) +[Consensys Best Practices](https://consensys.github.io/smart-contract-best-practices/known_attacks/) [DASP-Top 10](https://www.dasp.co/) -[SCSVS](https://github.com/securing/SCSVS) +[Solidity Documentation: Security Considerations](https://docs.soliditylang.org/en/latest/security-considerations.html) + +[Ethereum Wiki: Safety](https://github.com/ethereum/wiki/wiki/Safety) + +[Trail of Bits Reference List](https://github.com/crytic/awesome-ethereum-security) + +[DeFi-Threat](https://github.com/manifoldfinance/defi-threat) + +[Runtimeverification - List-of-Security-Vulnerabilties](https://github.com/runtimeverification/verified-smart-contracts/wiki/List-of-Security-Vulnerabilities) diff --git a/attack vectors/Frontrunning.md b/attack vectors/Frontrunning.md deleted file mode 100644 index 8878f35..0000000 --- a/attack vectors/Frontrunning.md +++ /dev/null @@ -1,5 +0,0 @@ -# Transaction Ordering / Frontrunning - -Miners can see transactions for a short time before they get included in a block, and exploit this information. They can also alter the order of transactions within a block. Users also have some influence over this process by setting gas prices. This can often pose a security risk, especially in systems where users are bidding or otherwise competing for something. - -- [Sigmaprime: Block Timestamp Manipulation](https://blog.sigmaprime.io/solidity-security.html#block-timestamp) diff --git a/attack vectors/Honeypoy.md b/attack vectors/Honeypoy.md deleted file mode 100644 index 55438bb..0000000 --- a/attack vectors/Honeypoy.md +++ /dev/null @@ -1,3 +0,0 @@ -Honeypots are traps basically deployed to catch hold of the attacker. - -- [Honeypot | Hack Solidity #10](https://coinsbench.com/honeypot-hack-solidity-9-69bb7faddecd) diff --git a/attack vectors/Access_Control.md b/attack-vectors/Access_Control.md similarity index 100% rename from attack vectors/Access_Control.md rename to attack-vectors/Access_Control.md diff --git a/attack-vectors/Account_Existence_Check_for_low_level_calls.md b/attack-vectors/Account_Existence_Check_for_low_level_calls.md new file mode 100644 index 0000000..46ec8bf --- /dev/null +++ b/attack-vectors/Account_Existence_Check_for_low_level_calls.md @@ -0,0 +1,5 @@ +As written in the solidity documentation, the low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior to calling if needed. + +## Remediation + +Check before any low-level call that the address actually exists, for example before the low level call in the callERC20 function you can check that the address is a contract by checking its code size. diff --git a/attack-vectors/Arbitrary_Jumps_with_Function_Variables.md b/attack-vectors/Arbitrary_Jumps_with_Function_Variables.md new file mode 100644 index 0000000..1cbea81 --- /dev/null +++ b/attack-vectors/Arbitrary_Jumps_with_Function_Variables.md @@ -0,0 +1,5 @@ +Solidity supports function types. That is, a variable of function type can be assigned with a reference to a function with a matching signature. The function saved to such variable can be called just like a regular function. + +The problem arises when a user has the ability to arbitrarily change the function type variable and thus execute random code instructions. As Solidity doesn't support pointer arithmetics, it's impossible to change such variable to an arbitrary value. However, if the developer uses assembly instructions, such as `mstore` or assign operator, in the worst case scenario an attacker is able to point a function type variable to any code instruction, violating required validations and required state changes. + +- [SWC Registry: Arbitrary Jump with Function Type Variable](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-127) diff --git a/attack-vectors/Assert_Violation.md b/attack-vectors/Assert_Violation.md new file mode 100644 index 0000000..d071bc3 --- /dev/null +++ b/attack-vectors/Assert_Violation.md @@ -0,0 +1,5 @@ +Properly functioning code should never violate an `assert` statement. This can occur if developers mistakenly use `assert` instead of `require`. + +-[ SWC Registry: Assert Violation](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-110) + +- [SWC Registry: Assert Violation](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-123) -[Solidity Documentation: Error Handling](https://solidity.readthedocs.io/en/latest/control-structures.html?highlight%3Dassert#error-handling-assert-require-revert-and-exceptions) diff --git a/attack-vectors/Bypass_Contract_Size_Check.md b/attack-vectors/Bypass_Contract_Size_Check.md new file mode 100644 index 0000000..54bef11 --- /dev/null +++ b/attack-vectors/Bypass_Contract_Size_Check.md @@ -0,0 +1,3 @@ +It is possible to create a contract with code size returned by extcodesize equal to 0. This can lead to bypass of 0-address check in a contract, which may lead to lost of fund or locking of funds. + +- [Solidity By Example: Bypass Contract Size Check](https://solidity-by-example.org/hacks/contract-size/) diff --git a/attack-vectors/Code_With_No_Effects.md b/attack-vectors/Code_With_No_Effects.md new file mode 100644 index 0000000..52b491d --- /dev/null +++ b/attack-vectors/Code_With_No_Effects.md @@ -0,0 +1,3 @@ +In Solidity, it's possible to write code that does not produce the intended effects. Currently, the solidity compiler will not return a warning for effect-free code. This can lead to the introduction of "dead" code that does not properly performing an intended action. + +For example, it's easy to miss the trailing parentheses in `msg.sender.call.value(address(this).balance)("");`, which could lead to a function proceeding without transferring funds to msg.sender. Although, this should be avoided by checking the return value of the call. diff --git a/attack-vectors/Complex_Modifiers.md b/attack-vectors/Complex_Modifiers.md new file mode 100644 index 0000000..70cb4f0 --- /dev/null +++ b/attack-vectors/Complex_Modifiers.md @@ -0,0 +1,3 @@ +Use modifiers only for input validation with `require`. Modifiers should not contain any substantive logic, because that logic will be executed before any input validation done at the start of function bodies. + +- [Consensys Best Practices: Use Modifiers Only for Assertions](https://consensys.github.io/smart-contract-best-practices/recommendations/#use-modifiers-only-for-assertions) diff --git a/attack-vectors/DOS.md b/attack-vectors/DOS.md new file mode 100644 index 0000000..eebacef --- /dev/null +++ b/attack-vectors/DOS.md @@ -0,0 +1,27 @@ +# DOS (Denial of Service) + +This is a broad category of attacks where an attacker may render a contract inoperable, temporarily or permanently. + +- [DASP: Denial of Service](https://www.dasp.co/#item-5) +- [Sigmaprime: Denial of Service](https://blog.sigmaprime.io/solidity-security.html#dos) +- [Trail of Bits: Denial of Service](https://github.com/crytic/not-so-smart-contracts/tree/master/denial_of_service) + +## Unexpected Revert + +An attacker may be able to exploit the fact that `transfer`(alternatively `require(addr.send(amount))`) reverts on failure to prevent a function from ever completing execution. + +- [Consensys Best Practices: DOS with Unexpected Revert](https://consensys.github.io/smart-contract-best-practices/attacks/denial-of-service/#dos-with-unexpected-revert) +- [SWC Registry: DOS with Failed Call](https://swcregistry.io/docs/SWC-113) + +## Block gas limit + +In cases where the users of a system can manipulate how much computation (gas) is necessary for the execution of some function, it may be possible to DOS the system by causing the required gas to exceed the block gas limit. This is often the case in systems that loop over an array or mapping that can be enlarged by users at little cost. + +- [Consensys Best Practices: DOS with Block Gas Limit](https://consensys.github.io/smart-contract-best-practices/attacks/denial-of-service/#dos-with-block-gas-limit) +- [SWC Registry: DOS with Block Gas Limit](https://swcregistry.io/docs/SWC-128) + +## External Calls without Gas Stipends + +In some cases, developers may want to make a transfer and continue execution regardless of the result. One way to achieve this is with `call.value(v)()`, however this may allow the recipient to consume all the gas of the calling function, preventing execution from continuing. See example 1 here: + +- [Sigmaprime: Denial of Service](https://blog.sigmaprime.io/solidity-security.html#dos) diff --git a/attack-vectors/Dirty_Higher_Order_Bits.md b/attack-vectors/Dirty_Higher_Order_Bits.md new file mode 100644 index 0000000..5213265 --- /dev/null +++ b/attack-vectors/Dirty_Higher_Order_Bits.md @@ -0,0 +1,3 @@ +If `keccak256(msg.data)` or some similar hash functionality is used (for example to log past calls), be aware that functions of types that don't occupy 32 bytes may be called with identical arguments but different hashes. + +- [Solidity Documentation: Minor Details](https://solidity.readthedocs.io/en/latest/security-considerations.html#minor-details) diff --git a/attack-vectors/Entropy_Illusion.md b/attack-vectors/Entropy_Illusion.md new file mode 100644 index 0000000..87c69b6 --- /dev/null +++ b/attack-vectors/Entropy_Illusion.md @@ -0,0 +1,5 @@ +The EVM does not have support for uncertainty/random number generation. Contracts may try to simulate uncertainty in a way that is in fact predictable and exploitable. + +- [Sigmaprime: Entropy Illusion](https://blog.sigmaprime.io/solidity-security.html#entropy) +- [DASP: Bad Randomness](https://www.dasp.co/#item-6) +- [SWC Registry: Weak Sources of Randomness from Chain Attributes](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-120) diff --git a/attack-vectors/Experimental_Language_Features.md b/attack-vectors/Experimental_Language_Features.md new file mode 100644 index 0000000..ed80f74 --- /dev/null +++ b/attack-vectors/Experimental_Language_Features.md @@ -0,0 +1 @@ +Avoid experimental language features, as these are not properly tested and have contained [vulnerabilities](https://blog.ethereum.org/2019/03/26/solidity-optimizer-and-abiencoderv2-bug) in the past. diff --git a/attack-vectors/External_Contrac_Referencing.md b/attack-vectors/External_Contrac_Referencing.md new file mode 100644 index 0000000..8a61728 --- /dev/null +++ b/attack-vectors/External_Contrac_Referencing.md @@ -0,0 +1,3 @@ +When a contract delegates some of its functionality to an external contract whose address is either inaccessible or subject to change, a benign implementation may be swapped out for a malicious one. + +- [Sigmaprime: External Contract Referencing](https://blog.sigmaprime.io/solidity-security.html#contract-reference) diff --git a/attack vectors/Flash_Loan_Attack.md b/attack-vectors/Flash_Loan_Attack.md similarity index 100% rename from attack vectors/Flash_Loan_Attack.md rename to attack-vectors/Flash_Loan_Attack.md diff --git a/attack-vectors/Floating_Point_Arithmetic.md b/attack-vectors/Floating_Point_Arithmetic.md new file mode 100644 index 0000000..4317d03 --- /dev/null +++ b/attack-vectors/Floating_Point_Arithmetic.md @@ -0,0 +1,3 @@ +Fixed point numbers are not yet fully supported by solidity. User implementations may contain errors. + +- [Fixed point numbers are not yet fully supported by solidity. User implementations may contain errors.](https://blog.sigmaprime.io/solidity-security.html#precision) diff --git a/attack-vectors/Force_Feeding.md b/attack-vectors/Force_Feeding.md new file mode 100644 index 0000000..0eab824 --- /dev/null +++ b/attack-vectors/Force_Feeding.md @@ -0,0 +1,82 @@ +Forcing a smart contract to hold an Ether balance can influence its internal accounting and security assumptions. +There are multiple ways a smart contract can receive Ether. The hierarchy is as follows: + +1. Check whether a payable external `receive` function is defined. +2. If not, check whether a payable external `fallback` function is defined. +3. Revert. + +The precedence of each function is explained in this great graphic from the [Solidity by Example](https://solidity-by-example.org/sending-ether/) article: + +``` +Which function is called, fallback() or receive()? + + send Ether + | + msg.data is empty? + / \ + yes no + / \ +receive() exists? fallback() + / \ + yes no + / \ + receive() fallback() +``` + +Consider the following example: + +```sol +pragma solidity ^0.8.13; + +contract Vulnerable { + receive() external payable { + revert(); + } + + function somethingBad() external { + require(address(this).balance > 0); + // Do something bad + } +} +``` + +The contract's logic seemingly disallows direct payments and prevents "something bad" from happening. +However, calling `revert` in both `fallback` and `receive` **cannot prevent the contract from receiving Ether**. +The following techniques can be used to force-feed Ether to a smart contract. + +### Selfdestruct + +When the `SELFDESTRUCT` opcode is called, funds of the calling address are sent to the address on the stack, and execution is immediately halted. +Since this opcode works on the EVM-level, Solidity-level functions that might block the receipt of Ether [will not be executed](https://solidity.readthedocs.io/en/develop/security-considerations.html#sending-and-receiving-ether). + +### Pre-calculated Deployments + +Additionally, the target address of newly deployed smart contracts is generated in a deterministic fashion. +The address generation can be looked up in any EVM implementation, such as the [py-evm reference implementation](https://github.com/ethereum/py-evm/blob/e924f63992a35212616b4e20355d161bc4348925/eth/_utils/address.py#L17-L18) by the Ethereum Foundation: + +```python +def generate_contract_address(address: Address, nonce: int) -> Address: + return force_bytes_to_address(keccak(rlp.encode([address, nonce]))) +``` + +An attacker can send funds to this address before the deployment has happened. +This is also illustrated by [this 2017 Underhanded Solidity Contest submission](https://github.com/Arachnid/uscc/tree/master/submissions-2017/ricmoo). + +### Block Rewards and Coinbase + +Depending on the attacker's capabilities, they can also start proof-of-work mining. +By setting the target address to their `coinbase`, block rewards will be added to its balance. +As this is yet another EVM-level capability, checks performed by Solidity are ineffective. + +### Solution + +The above effects illustrate that relying on exact comparisons to the contract's Ether balance is unreliable. +The smart contract's business logic must consider that the actual balance associated with it can be higher than the internal accounting's value. + +**In general, we strongly advise against using the contract's balance as a guard.** + +More information can be found in [SWC-132](https://swcregistry.io/docs/SWC-132). + +## References + +- [Force Feeding | Consensys](https://consensys.github.io/smart-contract-best-practices/attacks/force-feeding/#) diff --git a/attack-vectors/Frontend_(Off_Chain)_Attacks.md b/attack-vectors/Frontend_(Off_Chain)_Attacks.md new file mode 100644 index 0000000..893d47e --- /dev/null +++ b/attack-vectors/Frontend_(Off_Chain)_Attacks.md @@ -0,0 +1,10 @@ +These are possible vulnerabilities in frontends to ethereum contracts, not vulnerabilities in the contracts themselves. (Possibly out of scope.) + +## Short Address Attack + +Short address attacks are a side-effect of the EVM itself accepting incorrectly padded arguments. Attackers can exploit this by using specially-crafted addresses to make poorly coded clients encode arguments incorrectly before including them in transactions. Is this an EVM issue or a client issue? Should it be fixed in smart contracts instead? While everyone has a different opinion, the fact is that a great deal of ether could be directly impacted by this issue. While this vulnerability has yet to be exploited in the wild, it is a good demonstration of problems arising from the interaction between clients and the Ethereum blockchain. Other off-chain issues exist: an important one is the Ethereum ecosystem's deep trust in specific Javascript front ends, browser plugins and public nodes. An infamous off-chain exploit was used in the hack of the Coindash ICO that modified the company's Ethereum address on their webpage to trick participants into sending ethers to the attacker's address. + +Frontends should validate any input used to make transactions on chain. + +- [Sigmaprime: Short Address / Parameter Attack](https://blog.sigmaprime.io/solidity-security.html#short-address) +- [DASP: Short Address Attack](https://www.dasp.co/#item-9) diff --git a/attack-vectors/Function_Selector_Abuse.md b/attack-vectors/Function_Selector_Abuse.md new file mode 100644 index 0000000..3d340fb --- /dev/null +++ b/attack-vectors/Function_Selector_Abuse.md @@ -0,0 +1,11 @@ +To call a function on another contract, the standard ABI way to do so is to pass as calldata a function "selector", followed by the encoded arguments. You can read more [here](https://docs.soliditylang.org/en/v0.8.7/abi-spec.html), but a short example follows. + +In solidity, a call may look like `otherContract.foo("hello")`, but in reality, the call becomes `address(otherContract).call(abi.encodeWithSignature("foo(string)", "hello"))` which in practice becomes + +`0xf31a69690000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000568656c6c6f000000000000000000000000000000000000000000000000000000` + +The first 4 bytes `0xf31a6969`, are called the function selector, and consist of the first 4 bytes of the Keccak256 hash of the string "foo(string)". All ABI-compliant contracts on Ethereum begin by looking at these bytes of the calldata and jump to the corresponding function body. + +If a contract performs a call to an external contract, and the user can influence *any* part of the method signature (such as the function name, or its type), they can call *any* function on the external contract, simply by manipulating the string until a selector matching the desired one is found. + +This was behind the Poly Network hack in August 2021, where an attacker crafted messages on one chain which got processed on another chain. The contracts assumed well-behaved transactions, and the attacker managed to trick the contracts into calling privileged functions on yet other contracts. diff --git a/attack-vectors/Griefing.md b/attack-vectors/Griefing.md new file mode 100644 index 0000000..d8751bf --- /dev/null +++ b/attack-vectors/Griefing.md @@ -0,0 +1,65 @@ +This attack may be possible on a contract which accepts generic data and uses it to make a call +another contract (a 'sub-call') via the low level `address.call()` function, as is often the case +with multisignature and transaction relayer contracts. + +If the call fails, the contract has two options: + +1. revert the whole transaction +1. continue execution. + +Take the following example of a simplified `Relayer` contract which continues execution regardless +of the outcome of the subcall: + +```sol +contract Relayer { + mapping (bytes => bool) executed; + + function relay(bytes _data) public { + // replay protection; do not call the same transaction twice + require(executed[_data] == 0, "Duplicate call"); + executed[_data] = true; + innerContract.call(bytes4(keccak256("execute(bytes)")), _data); + } +} +``` + +This contract allows transaction relaying. Someone who wants to make a transaction but can't +execute it by himself (e.g. due to the lack of ether to pay for gas) can sign data that he wants to +pass and transfer the data with his signature over any medium. A third party "forwarder" can then +submit this transaction to the network on behalf of the user. + +If given just the right amount of gas, the `Relayer` would complete execution recording the +`_data`argument in the `executed` mapping, but the subcall would fail because it received +insufficient gas to complete execution. + +!!! Note +When a contract makes a sub-call to another contract, the EVM limits the gas forwarded to +[to 63/64 of the remaining gas](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-150.md), + +An attacker can use this to censor transactions, causing them to fail by sending them with a low +amount of gas. This attack is a form of "[griefing](https://en.wikipedia.org/wiki/Griefer)": It +doesn't directly benefit the attacker, but causes grief for the victim. A dedicated attacker, +willing to consistently spend a small amount of gas could theoretically censor all transactions +this way, if they were the first to submit them to `Relayer`. + +One way to address this is to implement logic requiring forwarders to provide enough gas to finish +the subcall. If the miner tried to conduct the attack in this scenario, the `require` statement +would fail and the inner call would revert. A user can specify a minimum gasLimit along with the +other data (in this example, typically the `_gasLimit` value would be verified by a signature, but +that is omitted for simplicity in this case). + +```sol +// contract called by Relayer +contract Executor { + function execute(bytes _data, uint _gasLimit) { + require(gasleft() >= _gasLimit); + ... + } +} +``` + +Another solution is to permit only trusted accounts to relay the transaction. + +## References - + +- [Griefing | Consensys](https://consensys.github.io/smart-contract-best-practices/attacks/griefing/) diff --git a/attack vectors/Hidden_malicious_code.md b/attack-vectors/Hidden_malicious_code.md similarity index 100% rename from attack vectors/Hidden_malicious_code.md rename to attack-vectors/Hidden_malicious_code.md diff --git a/attack-vectors/Historic_Attacks.md b/attack-vectors/Historic_Attacks.md new file mode 100644 index 0000000..3b975f4 --- /dev/null +++ b/attack-vectors/Historic_Attacks.md @@ -0,0 +1,19 @@ +### Constructor Names + +Fixed in solidity `v0.4.22` + +- [Sigmaprime: Constructors with Care](https://blog.sigmaprime.io/solidity-security.html#constructors) +- [Trail of Bits: Wrong Constructor Name](https://github.com/crytic/not-so-smart-contracts/tree/master/wrong_constructor_name) +- [SWC Registry: Incorrect Constructor Name](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-118) + +### Call Depth Attack + +Fixed in [EIP 150](https://github.com/ethereum/EIPs/issues/150) + +- [Solidity Documentation: Callstack Depth](https://solidity.readthedocs.io/en/latest/security-considerations.html#callstack-depth) + +### Solidity Abi Encoder v2 Bug + +Fixed in solidity `v0.5.7` + +- [Ethereum.org Blog](https://blog.ethereum.org/2019/03/26/solidity-optimizer-and-abiencoderv2-bug/) diff --git a/attack-vectors/Improper_Array_Deletion.md b/attack-vectors/Improper_Array_Deletion.md new file mode 100644 index 0000000..8f1cf3f --- /dev/null +++ b/attack-vectors/Improper_Array_Deletion.md @@ -0,0 +1,7 @@ +In Solidity, we remove an element from an array using the “delete” function. However, the length and sequence of the array may not remain as expected. If we delete the array index x, we will see that the array length remains the same, just that the value at index x has been set to zero. + +## Remediation: + +Instead of “delete” and “.length=0”, use push and pop functions to interact with array elements. + +- [Improper Array Deletion](https://blog.solidityscan.com/improper-array-deletion-82672eed8e8d) diff --git a/attack-vectors/Incorrect_Interface.md b/attack-vectors/Incorrect_Interface.md new file mode 100644 index 0000000..5ae92ea --- /dev/null +++ b/attack-vectors/Incorrect_Interface.md @@ -0,0 +1,3 @@ +A contract interface defines functions with a different type signature than the implementation, causing two different method id's to be created. As a result, when the interfact is called, the fallback method will be executed. + +- [Trail of Bits: Incorrect Interfaces](https://github.com/crytic/not-so-smart-contracts/tree/master/incorrect_interface) diff --git a/attack-vectors/Insufficient_Gas_Attacks.md b/attack-vectors/Insufficient_Gas_Attacks.md new file mode 100644 index 0000000..236ae30 --- /dev/null +++ b/attack-vectors/Insufficient_Gas_Attacks.md @@ -0,0 +1 @@ +If a function makes a call to an external subroutine with `call`, an attacker may be able to cause the function to only partially execute by sending a (precise) insufficient amount of gas. diff --git a/attack-vectors/Integer_Arithmetic.md b/attack-vectors/Integer_Arithmetic.md new file mode 100644 index 0000000..212e18a --- /dev/null +++ b/attack-vectors/Integer_Arithmetic.md @@ -0,0 +1,6 @@ +Neither the EVM nor Solidity (before v0.8) provide builtin error reporting for arithmetic overflow/underflow. Consequently, applications need to check for these cases themselves. Furthermore, one cannot make the (seemingly reasonable) assumption that `x != -x`, because of [this case](https://gist.github.com/endorphin/b9d78601930922aea12ed3ce6a286576). + +Note that since Solidity version 0.8, arithmetic operations revert on overflow and underflow. The developer can choose to bypass these checks by using the `unchecked` keyword, for example with `unchecked { x = a + b; }` + +- [SWC Registry: Integer Overflow and Underflow](https://smartcontractsecurity.github.io/swc-registry/docs/swc-101) +- [Trail of Bits: Integer Overflow](https://github.com/crytic/not-so-smart-contracts/tree/master/integer_overflow) diff --git a/attack-vectors/Loop_through_long_arrays.md b/attack-vectors/Loop_through_long_arrays.md new file mode 100644 index 0000000..9ae25e5 --- /dev/null +++ b/attack-vectors/Loop_through_long_arrays.md @@ -0,0 +1,3 @@ +If you are used to other programming languages you might be tempted to use arrays more than you actually should. + +Keep in mind that executing functions in Ethereum costs gas (money), and transactions have a gas limit by definition (the gas limit of a single block). If for some reason your smart contract uses a very long array, and at some point, you need to iterate through it, you might reach the gas limit making the function unexecutable. diff --git a/attack-vectors/Message_call_with_hardcoded_gas_amount.md b/attack-vectors/Message_call_with_hardcoded_gas_amount.md new file mode 100644 index 0000000..fe0d798 --- /dev/null +++ b/attack-vectors/Message_call_with_hardcoded_gas_amount.md @@ -0,0 +1 @@ +The `transfer()` and `send()` functions forward a fixed amount of 2300 gas. Historically, it has often been recommended to use these functions for value transfers to guard against reentrancy attacks. However, the gas cost of EVM instructions may change significantly during hard forks which may break already deployed contract systems that make fixed assumptions about gas costs. For example. [EIP 1884](https://eips.ethereum.org/EIPS/eip-1884) broke several existing smart contracts due to a cost increase in the SLOAD instruction. diff --git a/attack-vectors/Miners_Attack.md b/attack-vectors/Miners_Attack.md new file mode 100644 index 0000000..32433ed --- /dev/null +++ b/attack-vectors/Miners_Attack.md @@ -0,0 +1,16 @@ +# Miners Attack + +## Transaction Ordering / Frontrunning + +Since miners always get rewarded via gas fees for running code on behalf of externally owned addresses (EOA), users can specify higher fees to have their transactions mined more quickly. Since the Ethereum blockchain is public, everyone can see the contents of others' pending transactions. This means if a given user is revealing the solution to a puzzle or other valuable secret, a malicious user can steal the solution and copy their transaction with higher fees to preempt the original solution. If developers of smart contracts are not careful, this situation can lead to practical and devastating front-running attacks. + +- [Dasp: Front running](https://www.dasp.co/#item-7) +- [Sigmaprime: Race Conditions / Front-Running](https://blog.sigmaprime.io/solidity-security.html#race-conditions) +- [SWC Registry: Transaction Order Dependence](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-114) + +## Timestamp Manipulation + +Many contracts use block timestamps for various purposes. Keep in mind that miners can slightly adjust them to their advantage. + +- [Sigmaprime: Block Timestamp Manipulation](https://blog.sigmaprime.io/solidity-security.html#block-timestamp) +- [SWC Registry: Timestamp Dependence](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-116) diff --git a/attack-vectors/Offline_Owner.md b/attack-vectors/Offline_Owner.md new file mode 100644 index 0000000..4ef061d --- /dev/null +++ b/attack-vectors/Offline_Owner.md @@ -0,0 +1 @@ +Some systems may become inoperable if the owner or some other authority goes offline / loses their private key. This should be avoided. diff --git a/attack vectors/Oracle_Manipulation.md b/attack-vectors/Oracle_Manipulation.md similarity index 100% rename from attack vectors/Oracle_Manipulation.md rename to attack-vectors/Oracle_Manipulation.md diff --git a/attack-vectors/Outdated_Compiler.md b/attack-vectors/Outdated_Compiler.md new file mode 100644 index 0000000..69c9156 --- /dev/null +++ b/attack-vectors/Outdated_Compiler.md @@ -0,0 +1,4 @@ +Never use a compiler version that is significantly out of date. Implementations should specify an exact compiler version with pragma. + +- [SWC Registry: Outdated Compiler](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-102) +- [SWC Registry: Floating Pragma](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-103) diff --git a/attack-vectors/Payable_Multicall.md b/attack-vectors/Payable_Multicall.md new file mode 100644 index 0000000..23a7d12 --- /dev/null +++ b/attack-vectors/Payable_Multicall.md @@ -0,0 +1,5 @@ +This is present when Multicall is used on any contract which reads the value of `msg.value`. Multicall is a pattern to call several contract endpoints in one transaction, using `delegatecall`. A contract endpoint may implicitly assume that it is called in a single transaction, by looking at `msg.value`. Since the value is defined per transaction, calling the same endpoint twice in one multicall means that the same `msg.value` may be read several times, even though the value was only transferred once. + +For example, if a token contract accepts ETH in exchange for tokens in a `swap` function, and the contract implements multicall, an attacker may call swap several times in one transaction. Let's say the attacker sends along 1 ETH in the multicall transaction, which would normally give them 100 tokens. Each call to the `swap` function will read `msg.value` and transfer 100 tokens to the attacker. If the attacker calls `swap` 10 times in one multicall, they will get 1,000 tokens in exchange for 1 ETH. + +- [Uniswap v3 issue](https://github.com/Uniswap/v3-periphery/issues/52) diff --git a/attack-vectors/Precision_Loss_in_Calculations.md b/attack-vectors/Precision_Loss_in_Calculations.md new file mode 100644 index 0000000..0fc19a4 --- /dev/null +++ b/attack-vectors/Precision_Loss_in_Calculations.md @@ -0,0 +1 @@ +Smart contracts are high-level programs that are translated into EVM byte code and then deployed to the Ethereum blockchain for execution. Solidity mathematic procedures are similar to other programming languages. The following arithmetic operations are applicable to Solidity: Addition, Subtraction, Multiplication, Division (x / y), Modulus (x% y), Exponential (x\*\*y) In the case of performing Integer division, Solidity may truncate the result. Hence we must multiply before dividing to prevent such loss in precision diff --git a/attack-vectors/Privacy_Illusion.md b/attack-vectors/Privacy_Illusion.md new file mode 100644 index 0000000..b28534e --- /dev/null +++ b/attack-vectors/Privacy_Illusion.md @@ -0,0 +1,3 @@ +Developers may forget that everything on chain is public, and store private data in the open. + +- Checkout Ethernaut level for this diff --git a/attack-vectors/Proxy_Storage_Collision.md b/attack-vectors/Proxy_Storage_Collision.md new file mode 100644 index 0000000..9ec344d --- /dev/null +++ b/attack-vectors/Proxy_Storage_Collision.md @@ -0,0 +1 @@ +While using Proxy contract developers should be very carefull about position of storage variables as in Proxy contract the name of the varible doesn't matter but their position do. diff --git a/attack vectors/Reentrancy.md b/attack-vectors/Reentrancy.md similarity index 91% rename from attack vectors/Reentrancy.md rename to attack-vectors/Reentrancy.md index fd6ede1..15ecafa 100644 --- a/attack vectors/Reentrancy.md +++ b/attack-vectors/Reentrancy.md @@ -110,3 +110,9 @@ contract ReEntrancyGuard { ``` Example taken from [Solidity by Example](https://solidity-by-example.org/hacks/re-entrancy/) + +# + +- [SWC Registry: Reentrancy](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-107) +- [Sigmaprime: Reentrancy](https://blog.sigmaprime.io/solidity-security.html#reentrancy) +- [Trail of Bits: Reentrancy](https://github.com/crytic/not-so-smart-contracts/tree/master/reentrancy) diff --git a/attack-vectors/Right-To-Left-Override_control_character_(U+202E).md b/attack-vectors/Right-To-Left-Override_control_character_(U+202E).md new file mode 100644 index 0000000..db2fd56 --- /dev/null +++ b/attack-vectors/Right-To-Left-Override_control_character_(U+202E).md @@ -0,0 +1,7 @@ +Malicious actors can use the Right-To-Left-Override unicode character to force RTL text rendering and confuse users as to the real intent of a contract. + +## Remediation + +There are very few legitimate uses of the U+202E character. It should not appear in the source code of a smart contract. + +- [SWC Registry: Right-To-Left-Override control character (U+202E)](https://swcregistry.io/docs/SWC-130) diff --git a/attack vectors/Sandwich_Attach.md b/attack-vectors/Sandwich_Attack.md similarity index 100% rename from attack vectors/Sandwich_Attach.md rename to attack-vectors/Sandwich_Attack.md diff --git a/attack vectors/Signature_Replay.md b/attack-vectors/Signature_Replay.md similarity index 100% rename from attack vectors/Signature_Replay.md rename to attack-vectors/Signature_Replay.md diff --git a/attack-vectors/Unchecked_External_Calls.md b/attack-vectors/Unchecked_External_Calls.md new file mode 100644 index 0000000..3ec52f0 --- /dev/null +++ b/attack-vectors/Unchecked_External_Calls.md @@ -0,0 +1,7 @@ +In Solidity, there are multiple ways to call an external contract and send ether. The function `transfer` reverts if the transfer fails. However, the functions `call` and `send` return false. Programmers may mistakenly expect `call` and `send` to revert, and fail to check for their return value. + +- [DASP: Unchecked Low Level Calls](https://www.dasp.co/#item-4) + +- [Sigmaprime: Unchecked CALL Return Values](https://blog.sigmaprime.io/solidity-security.html#unchecked-calls) + +- [Trail of Bits: Unchecked External Calls](https://github.com/crytic/not-so-smart-contracts/tree/master/unchecked_external_call) diff --git a/attack-vectors/Uninitialized_Storage_Pointers.md b/attack-vectors/Uninitialized_Storage_Pointers.md new file mode 100644 index 0000000..5f44c32 --- /dev/null +++ b/attack-vectors/Uninitialized_Storage_Pointers.md @@ -0,0 +1,4 @@ +Local variables in solidity functions default to `storage` or `memory` depending on their type. Uninitialized local storage variables can point to unexpected storage locations in the contract, leading to intentional or unintentional vulnerabilities. + +- [Sigmaprime: Uninitialized Storage Pointers](https://blog.sigmaprime.io/solidity-security.html#storage) +- [SWC Registry: Uninitialized Storage Pointer](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-109) diff --git a/attack-vectors/Unprotected_Upgrades.md b/attack-vectors/Unprotected_Upgrades.md new file mode 100644 index 0000000..3e9219e --- /dev/null +++ b/attack-vectors/Unprotected_Upgrades.md @@ -0,0 +1,8 @@ +There is a lot that can go wrong with smart contract upgrades, and it would not make sense to list them here. 99% of the time, you should use OpenZeppelin’s upgraded plug-in tools for hardhat or truffle. But if you want a preview, this can help: + +1. Storage slots can clash. +2. Information stored via constructors or immutable variables won’t be available in the next contract. +3. Initializers need to be protected. +4. Selfdestruct can prevent upgrades. + +- [All you need to know about UPGRADABLE Smart Contracts](https://trustchain.medium.com/all-you-need-to-know-about-upgradable-proxies-865704a28bc7) diff --git a/attack vectors/Unsafe_Delegatecall.md b/attack-vectors/Unsafe_Delegatecall.md similarity index 100% rename from attack vectors/Unsafe_Delegatecall.md rename to attack-vectors/Unsafe_Delegatecall.md diff --git a/attack-vectors/Unused_Variable.md b/attack-vectors/Unused_Variable.md new file mode 100644 index 0000000..dcbd1f1 --- /dev/null +++ b/attack-vectors/Unused_Variable.md @@ -0,0 +1,7 @@ +Unused variables are allowed in Solidity and they do not pose a direct security issue. It is best practice though to avoid them as they can: + +- cause an increase in computations (and unnecessary gas consumption) +- indicate bugs or malformed data structures and they are generally a sign of poor code quality +- cause code noise and decrease readability of the code + +- [SWC Registry: Presence of unused variables](https://swcregistry.io/docs/SWC-131) diff --git a/attack-vectors/Use_of_Deprecated_Solidity_Functions.md b/attack-vectors/Use_of_Deprecated_Solidity_Functions.md new file mode 100644 index 0000000..5cfbd1b --- /dev/null +++ b/attack-vectors/Use_of_Deprecated_Solidity_Functions.md @@ -0,0 +1,3 @@ +Several functions and operators in Solidity are deprecated. Using them leads to reduced code quality. With new major versions of the Solidity compiler, deprecated functions and operators may result in side effects and compile errors. + +- [SWC Registry: Use of Deprecated Solidity Functions](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-111) diff --git a/attack-vectors/Variable_Shadowing.md b/attack-vectors/Variable_Shadowing.md new file mode 100644 index 0000000..b80fb5e --- /dev/null +++ b/attack-vectors/Variable_Shadowing.md @@ -0,0 +1,5 @@ +Variable shadowing occurs when a variable declared within a certain scope (decision block, method, or inner class) has the same name as a variable declared in an outer scope. If multiple variables with the same name are declared in different scopes, there may be unintended effects. This is easy to miss in the case where a contract inherits from a contract implemented in a separate file. + +- [Trail of Bits: Variable Shadowing](https://github.com/crytic/not-so-smart-contracts/tree/master/variable%20shadowing) +- [SWC Registry: Incorrect Inheritance Order](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-125) +- [SWC Registry: Shadowing State Variables](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-119) diff --git a/attack-vectors/Writes_to_Arbitrary_Storage_Locations.md b/attack-vectors/Writes_to_Arbitrary_Storage_Locations.md new file mode 100644 index 0000000..366f839 --- /dev/null +++ b/attack-vectors/Writes_to_Arbitrary_Storage_Locations.md @@ -0,0 +1,3 @@ +A smart contract's data (e.g., storing the owner of the contract) is persistently stored at some storage location (i.e., a key or address) on the EVM level. The contract is responsible for ensuring that only authorized user or contract accounts may write to sensitive storage locations. If an attacker is able to write to arbitrary storage locations of a contract, the authorization checks may easily be circumvented. This can allow an attacker to corrupt the storage; for instance, by overwriting a field that stores the address of the contract owner. + +- [SWC Registry: Write to Arbitrary Storage Location](https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-124) diff --git a/attack-vectors/Wrong_inheritance.md b/attack-vectors/Wrong_inheritance.md new file mode 100644 index 0000000..4ebd8c8 --- /dev/null +++ b/attack-vectors/Wrong_inheritance.md @@ -0,0 +1,5 @@ +Solidity supports multiple inheritance, meaning that one contract can inherit several contracts. Multiple inheritance introduces ambiguity called [Diamond Problem](https://en.wikipedia.org/wiki/Multiple_inheritance#The_diamond_problem): if two or more base contracts define the same function, which one should be called in the child contract? Solidity deals with this ambiguity by using reverse [C3 Linearization](https://en.wikipedia.org/wiki/C3_linearization), which sets a priority between base contracts. + +That way, base contracts have different priorities, so the order of inheritance matters. Neglecting inheritance order can lead to unexpected behavior. + +- [SWC Registry: Incorrect Inheritance Order](https://swcregistry.io/docs/SWC-125)