diff --git a/not-so-smart-contracts/README.md b/not-so-smart-contracts/README.md index c70b6c16..f2bbef10 100644 --- a/not-so-smart-contracts/README.md +++ b/not-so-smart-contracts/README.md @@ -1,6 +1,6 @@ # (Not So) Smart Contracts -This repository contains examples of common smart contract vulnerabilities, including code from real smart contracts. Use Not So Smart Contracts to learn about vulnerabilities, as a reference when performing security reviews, and as a benchmark for security and analysis tools: +This repository contains examples of common smart contract vulnerabilities, including code taken from actual smart contracts. Use Not So Smart Contracts as a learning resource for understanding vulnerabilities, as a reference during security reviews, and as a benchmark for security and analysis tools: - [Algorand](./algorand/README.md) - [Cairo](./cairo/README.md) diff --git a/not-so-smart-contracts/algorand/README.md b/not-so-smart-contracts/algorand/README.md index 19219b06..59fc764f 100644 --- a/not-so-smart-contracts/algorand/README.md +++ b/not-so-smart-contracts/algorand/README.md @@ -1,6 +1,6 @@ # (Not So) Smart Contracts -This repository contains examples of common Algorand smart contract vulnerabilities, including code from real smart contracts. Use Not So Smart Contracts to learn about Algorand vulnerabilities, as a reference when performing security reviews, and as a benchmark for security and analysis tools. +This repository contains examples of common Algorand smart contract vulnerabilities, including code from real smart contracts. Use the Not So Smart Contracts to learn about Algorand vulnerabilities, as a reference when performing security reviews, and as a benchmark for security and analysis tools. ## Features @@ -8,26 +8,26 @@ Each _Not So Smart Contract_ includes a standard set of information: - Description of the vulnerability type - Attack scenarios to exploit the vulnerability -- Recommendations to eliminate or mitigate the vulnerability -- Real-world contracts that exhibit the flaw +- Recommendations for eliminating or mitigating the vulnerability +- Real-world contracts exhibiting the flaw - References to third-party resources with more information ## Vulnerabilities -| Not So Smart Contract | Description | Applicable to smart signatures | Applicable to smart contracts | -| ------------------------------------------------------- | ------------------------------------------------------------------------------ | ------------------------------ | ----------------------------- | -| [Rekeying](rekeying) | Attacker rekeys an account | yes | yes | -| [Unchecked Transaction Fees](unchecked_transaction_fee) | Attacker sets excessive fees for smart signature transactions | yes | no | -| [Closing Account](closing_account) | Attacker closes smart signature accounts | yes | no | -| [Closing Asset](closing_asset) | Attacker transfers entire asset balance of a smart signature | yes | no | -| [Group Size Check](group_size_check) | Contract does not check transaction group size | yes | yes | -| [Time-based Replay Attack](time_based_replay_attack) | Contract does not use lease for periodic payments | yes | no | -| [Access Controls](access_controls) | Contract does not enfore access controls for updating and deleting application | no | yes | -| [Asset Id Check](asset_id_check) | Contract does not check asset id for asset transfer operations | yes | yes | -| [Denial of Service](denial_of_service) | Attacker stalls contract execution by opting out of a asset | yes | yes | +| Not So Smart Contract | Description | Applicable to smart signatures | Applicable to smart contracts | +| ------------------------------------------------------- | -------------------------------------------------------------------------------- | ------------------------------ | ----------------------------- | +| [Rekeying](rekeying) | Attacker rekeys an account | yes | yes | +| [Unchecked Transaction Fees](unchecked_transaction_fee) | Attacker sets excessive fees for smart signature transactions | yes | no | +| [Closing Account](closing_account) | Attacker closes smart signature accounts | yes | no | +| [Closing Asset](closing_asset) | Attacker transfers the entire asset balance of a smart signature | yes | no | +| [Group Size Check](group_size_check) | Contract does not check transaction group size | yes | yes | +| [Time-based Replay Attack](time_based_replay_attack) | Contract does not use lease for periodic payments | yes | no | +| [Access Controls](access_controls) | Contract does not enforce access controls for updating and deleting applications | no | yes | +| [Asset Id Check](asset_id_check) | Contract does not check asset id for asset transfer operations | yes | yes | +| [Denial of Service](denial_of_service) | Attacker stalls contract execution by opting out of an asset | yes | yes | ## Credits These examples are developed and maintained by [Trail of Bits](https://www.trailofbits.com/). -If you have questions, problems, or just want to learn more, then join the #ethereum channel on the [Empire Hacking Slack](https://empireslacking.herokuapp.com/) or [contact us](https://www.trailofbits.com/contact/) directly. +If you have questions, issues, or simply want to learn more, join the #ethereum channel on the [Empire Hacking Slack](https://empireslacking.herokuapp.com/) or [contact us](https://www.trailofbits.com/contact/) directly. diff --git a/not-so-smart-contracts/algorand/access_controls/README.md b/not-so-smart-contracts/algorand/access_controls/README.md index f743d0aa..6265996b 100644 --- a/not-so-smart-contracts/algorand/access_controls/README.md +++ b/not-so-smart-contracts/algorand/access_controls/README.md @@ -1,18 +1,17 @@ # Access Controls -Lack of appropriate checks for application calls of type UpdateApplication and DeleteApplication allows attackers to update application’s code or delete an application entirely. +Inadequate checks for application calls of type UpdateApplication and DeleteApplication can allow attackers to alter an application's code or delete it entirely. ## Description -When an application call is successful, additional operations are executed based on the OnComplete field. If the OnComplete field is set to UpdateApplication the approval and clear programs of the application are replaced with the programs specified in the transaction. Similarly, if the OnComplete field is set to DeleteApplication, application parameters are deleted. -This allows attackers to update or delete the application if proper access controls are not enforced in the application. +When an application call is successful, further operations are carried out based on the OnComplete field. If the OnComplete field is set to UpdateApplication, the approval and clear programs of the application get replaced with the programs specified in the transaction. Likewise, if the OnComplete field is set to DeleteApplication, the application parameters get deleted. This can let attackers update or delete the application if proper access controls are not in place. ## Exploit Scenarios -A stateful contract serves as a liquidity pool for a pair of tokens. Users can deposit the tokens to get the liquidity tokens and can get back their funds with rewards through a burn operation. The contract does not enforce restrictions for UpdateApplication type application calls. Attacker updates the approval program with a malicious program that transfers all assets in the pool to the attacker's address. +A stateful contract functions as a liquidity pool for two tokens. Users can deposit these tokens to acquire liquidity tokens and can retrieve their funds along with rewards via a burn operation. The contract, however, does not enforce any restrictions for UpdateApplication type application calls. An attacker can update the approval program with a malicious program that transfers all the assets in the pool to their address. ## Recommendations -- Set proper access controls and apply various checks before approving applications calls of type UpdateApplication and DeleteApplication. +- Implement appropriate access controls and conduct various checks before approving application calls of type UpdateApplication and DeleteApplication. -- Use [Tealer](https://github.com/crytic/tealer) to detect this issue. +- Utilize [Tealer](https://github.com/crytic/tealer) to identify this issue. diff --git a/not-so-smart-contracts/algorand/asset_id_check/README.md b/not-so-smart-contracts/algorand/asset_id_check/README.md index 01c59330..f0699459 100644 --- a/not-so-smart-contracts/algorand/asset_id_check/README.md +++ b/not-so-smart-contracts/algorand/asset_id_check/README.md @@ -1,15 +1,15 @@ -# Asset Id Check +# Asset ID Check -Lack of verification of asset id in the contract allows attackers to transfer a different asset in place of the expected asset and mislead the application. +Failing to verify the asset ID in a contract can allow attackers to transfer a different asset instead of the expected one, potentially misleading the application. ## Description -Contracts accepting and doing operations based on the assets transferred to the contract must verify that the transferred asset is the expected asset by checking the asset Id. Absence of check for expected asset Id could allow attackers to manipulate contract’s logic by transferring a fake, less or more valuable asset instead of the correct asset. +Contracts that accept and perform operations based on assets transferred to them must verify that the transferred asset is indeed the expected asset by checking the asset ID. Neglecting to check the expected asset ID could enable attackers to manipulate the contract's logic by transferring a fake, less valuable, or more valuable asset instead of the correct one. ## Exploit Scenarios -- A liquidity pool contract mints liquidity tokens on deposit of two tokens. Contract does not check that the asset Ids in the two asset transfer transactions are correct. Attacker deposits the same less valuable asset in the two transactions and withdraws both tokens by burning the pool tokens. -- User creates a delegate signature that allows recurring transfers of a certain asset. Attacker creates a valid asset transfer transaction of more valuable assets. +- A liquidity pool contract mints liquidity tokens upon the deposit of two tokens. The contract does not verify that the asset IDs in the two asset transfer transactions are correct. The attacker deposits the same, less valuable asset in both transactions and withdraws both tokens by burning the pool tokens. +- A user creates a delegate signature that permits recurring transfers of a certain asset. The attacker then generates a valid asset transfer transaction involving more valuable assets. ## Examples @@ -35,4 +35,4 @@ def withdraw_asset( ## Recommendations -Verify the asset id to be expected asset for all asset related operations in the contract. +Ensure that the asset ID is verified as the expected asset for all asset-related operations in the contract. diff --git a/not-so-smart-contracts/algorand/closing_account/README.md b/not-so-smart-contracts/algorand/closing_account/README.md index 4dc8a7f0..2041589d 100644 --- a/not-so-smart-contracts/algorand/closing_account/README.md +++ b/not-so-smart-contracts/algorand/closing_account/README.md @@ -1,20 +1,20 @@ -# Closing Account +# Closing Accounts -Lack of check for CloseRemainderTo transaction field in smart signatures allows attackers to transfer entire funds of the contract account or the delegator’s account to their account. +A lack of checks for the CloseRemainderTo transaction field in smart signatures allows attackers to transfer the entire funds of a contract account or a delegator's account to their own account. ## Description -Algorand accounts must satisfy minimum balance requirement and protocol rejects transactions whose execution results in account balance lower than the required minimum. In order to transfer the entire balance and close the account, users should use the CloseRemainderTo field of a payment transaction. Setting the CloseRemainderTo field transfers the entire account balance remaining after transaction execution to the specified address. +Algorand accounts must meet the minimum balance requirement, and the protocol rejects transactions if their execution would result in an account balance lower than the required minimum. To transfer the entire balance and close an account, users should use the CloseRemainderTo field of a payment transaction. Setting the CloseRemainderTo field transfers the remaining account balance after transaction execution to the specified address. -Any user with access to the smart signature may construct and submit the transactions using the smart signature. The smart signatures approving payment transactions have to ensure that the CloseRemainderTo field is set to the ZeroAddress or any other specific address to avoid unintended transfer of funds. +Any user with access to the smart signature can construct and submit transactions using that smart signature. To avoid unintended fund transfers, smart signatures approving payment transactions must ensure that the CloseRemainderTo field is set to the ZeroAddress or another specific address. ## Exploit Scenarios -A user creates a delegate signature for recurring payments. Attacker creates a valid transaction and sets the CloseRemainderTo field to their address. +A user creates a delegate signature for recurring payments. An attacker creates a valid transaction and sets the CloseRemainderTo field to their own address. ## Examples -Note: This code contains several other vulnerabilities, see [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), [Time-based Replay Attack](../time_based_replay_attack). +Note: This code contains several other vulnerabilities. See [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), [Time-based Replay Attack](../time_based_replay_attack). ```py def withdraw( @@ -36,4 +36,4 @@ def withdraw( ## Recommendations -Verify that the CloseRemainderTo field is set to the ZeroAddress or to any intended address before approving the transaction in the Teal contract. +Before approving a transaction in the Teal contract, verify that the CloseRemainderTo field is set to the ZeroAddress or another intended address. diff --git a/not-so-smart-contracts/algorand/closing_asset/README.md b/not-so-smart-contracts/algorand/closing_asset/README.md index 4b3a2ec1..301a5a10 100644 --- a/not-so-smart-contracts/algorand/closing_asset/README.md +++ b/not-so-smart-contracts/algorand/closing_asset/README.md @@ -1,20 +1,20 @@ # Closing Asset -Lack of check for AssetCloseTo transaction field in smart signatures allows attackers to transfer the entire asset balance of the contract account or the delegator’s account to their account. +A lack of checks for the AssetCloseTo transaction field in smart signatures enables attackers to transfer the entire asset balance of the contract account or the delegator's account to their account. ## Description -Algorand supports Fungible and Non Fungible Tokens using Algorand Standard Assets(ASA). An Algorand account must first opti-in to the asset before that account can receive any tokens. Opting to an asset increases the minimum balance requirement of the account. Users can opt-out of the asset and decrease the minimum balance requirement using the AssetCloseTo field of Asset Transfer transaction. Setting the AssetCloseTo field transfers the account’s entire token balance remaining after transaction execution to the specified address. +Algorand supports Fungible and Non-Fungible Tokens using Algorand Standard Assets (ASA). An Algorand account must first opt-in to the asset before it can receive any tokens. Opting-in to an asset increases the minimum balance requirement of the account. Users can opt-out of the asset and decrease the minimum balance requirement by using the AssetCloseTo field of the Asset Transfer transaction. Setting the AssetCloseTo field transfers the account's entire token balance remaining after the transaction execution to the specified address. -Any user with access to the smart signature may construct and submit the transactions using the smart signature. The smart signatures approving asset transfer transactions have to ensure that the AssetCloseTo field is set to the ZeroAddress or any other specific address to avoid unintended transfer of tokens. +Any user with access to the smart signature may construct and submit transactions using the smart signature. Smart signatures approving asset transfer transactions must ensure that the AssetCloseTo field is set to the ZeroAddress or any other specific address to prevent unintended token transfers. ## Exploit Scenarios -User creates a delegate signature that allows recurring transfers of a certain asset. Attacker creates a valid asset transfer transaction with AssetCloseTo field set to their address. +A user creates a delegate signature that allows recurring transfers of a certain asset. An attacker then creates a valid asset transfer transaction with the AssetCloseTo field set to their address. ## Examples -Note: This code contains several other vulnerabilities, see [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), [Closing Asset](../closing_asset), [Time-based Replay Attack](../time_based_replay_attack), [Asset Id Check](../asset_id_check). +Note: This code contains several other vulnerabilities, such as [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), [Closing Asset](../closing_asset), [Time-based Replay Attack](../time_based_replay_attack), and [Asset ID Check](../asset_id_check). ```py def withdraw_asset( @@ -36,4 +36,4 @@ def withdraw_asset( ## Recommendations -Verify that the AssetCloseTo field is set to the ZeroAddress or to the intended address before approving the transaction in the Teal contract. +Before approving the transaction in the Teal contract, make sure to verify that the AssetCloseTo field is set to the ZeroAddress or the intended address. diff --git a/not-so-smart-contracts/algorand/denial_of_service/README.md b/not-so-smart-contracts/algorand/denial_of_service/README.md index c1456e2c..8f638c81 100644 --- a/not-so-smart-contracts/algorand/denial_of_service/README.md +++ b/not-so-smart-contracts/algorand/denial_of_service/README.md @@ -1,18 +1,18 @@ # Denial of Service -When a contract does not verify whether an account has opted in to an asset and attempts to transfer that asset, an attacker can DoS other users if the contract's operation is to transfer asset to multiple accounts. +A denial of service (DoS) attack can occur when a contract does not verify whether an account has opted into an asset and then attempts to transfer that asset. If the contract's operation involves transferring an asset to multiple accounts, an attacker can use this weakness to launch a DoS attack against other users. ## Description -A user must explicitly opt-in to receive any particular Algorand Standard Asset(ASAs). A user may also opt out of an ASA. A transaction will fail if it attempts to transfer tokens to an account that didn’t opt in to that asset. This could be leveraged by attackers to DOS a contract if the contract’s operation depends on successful transfer of an asset to the attacker owned address. +A user must explicitly opt into receiving any particular Algorand Standard Asset (ASA). A user may also choose to opt out of an ASA. A transaction will fail if it attempts to transfer tokens to an account that has not opted into that asset. Attackers can exploit this vulnerability by launching a DoS attack on a contract if the contract's operation depends on a successful asset transfer to an attacker-owned address. ## Exploit Scenarios -Contract attempts to transfer assets to multiple users. One user is not opted in to the asset. The transfer operation fails for all users. +A contract tries to transfer assets to multiple users. However, one user has not opted into the asset, causing the transfer operation to fail for all users. ## Examples -Note: This code contains several other vulnerabilities, see [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), [Closing Asset](../closing_asset), [Group Size Check](../group_size_check), [Time-based Replay Attack](../time_based_replay_attack), [Asset Id Check](../asset_id_check) +Note: This code contains several other vulnerabilities. See [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), [Closing Asset](../closing_asset), [Group Size Check](../group_size_check), [Time-based Replay Attack](../time_based_replay_attack), and [Asset ID Check](../asset_id_check) ```py def split_and_withdraw_asset( @@ -37,4 +37,4 @@ def split_and_withdraw_asset( ## Recommendations -Use pull over push pattern for transferring assets to users. +Employ the pull-over-push pattern for transferring assets to users. diff --git a/not-so-smart-contracts/algorand/group_size_check/README.md b/not-so-smart-contracts/algorand/group_size_check/README.md index 3d21f9d5..b55b93d7 100644 --- a/not-so-smart-contracts/algorand/group_size_check/README.md +++ b/not-so-smart-contracts/algorand/group_size_check/README.md @@ -1,18 +1,18 @@ # Group Size Check -Lack of group size check in contracts that are supposed to be called in an atomic group transaction might allow attackers to misuse the application. +A lack of group size check in contracts intended for atomic group transactions might allow attackers to misuse the application. ## Description -Algorand supports atomic transfers, an atomic transfer is a group of transactions that are submitted and processed as a single transaction. A group can contain upto 16 transactions and the group transaction fails if any of the included transactions fails. Algorand applications make use of group transactions to realize operations that may not be possible using a single transaction model. In such cases, it is necessary to check that the group transaction in itself is valid along with the individual transactions. One of the checks whose absence could be misused is group size check. +Algorand supports atomic transfers, which are groups of transactions that are submitted and processed as a single transaction. A group can contain up to 16 transactions, and the group transaction fails if any of the included transactions fail. Algorand applications utilize group transactions to perform operations that might not be possible using a single transaction model. In such cases, it is necessary to verify that the group transaction itself is valid, along with the individual transactions. One check that, if absent, could be exploited is the group size check. ## Exploit Scenarios -Application only checks that transactions at particular indices are meeting the criteria and performs the operations based on that. Attackers can create the transactions at the checked indices correctly and include equivalent application call transactions at all the remaining indices. Each application call executes successfully as every execution checks the same set of transactions. This results in performing operations multiple times, once for each application call. This could be damaging if those operations include funds or assets transfers among others. +An application might only verify that transactions at particular indices meet the criteria and perform operations based on that. Attackers can create the transactions at the checked indices correctly and include equivalent application call transactions at all remaining indices. Each application call executes successfully since every execution checks the same set of transactions. This results in performing operations multiple times: once for each application call. This could be damaging if those operations involve funds or asset transfers, among other things. ## Examples -Note: This code contains several other vulnerabilities, see [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), [Closing Account](../closing_account), [Time-based Replay Attack](../time_based_replay_attack). +Note: This code contains several other vulnerabilities, see [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), [Closing Account](../closing_account), and [Time-based Replay Attack](../time_based_replay_attack). ```py def split_and_withdraw( @@ -37,8 +37,8 @@ def split_and_withdraw( ## Recommendations -- Verify that the group size of an atomic transfer is the intended size in the contracts. +- Ensure that contracts verify the intended group size of an atomic transfer. - Use [Tealer](https://github.com/crytic/tealer) to detect this issue. -- Favor using ABI for smart contracts and relative indexes to verify the group transaction. +- Favor using an Application Binary Interface (ABI) for smart contracts and relative indexes to verify group transactions. diff --git a/not-so-smart-contracts/algorand/rekeying/README.md b/not-so-smart-contracts/algorand/rekeying/README.md index d1da6d93..e9ca9f7f 100644 --- a/not-so-smart-contracts/algorand/rekeying/README.md +++ b/not-so-smart-contracts/algorand/rekeying/README.md @@ -1,27 +1,26 @@ # Rekeying -The lack of check for RekeyTo field in the Teal program allows malicious actors to rekey the associated account and control the account assets directly, bypassing the restrictions imposed by the Teal contract. +A lack of verification for the RekeyTo field in a Teal program may allow malicious actors to rekey the associated account and control its assets directly, bypassing the restrictions imposed by the Teal contract. ## Description -Rekeying is an Algorand feature which allows a user to transfer the authorization power of their account to a different account. When an account has been rekeyed, all the future transactions from that account are accepted by the blockchain, if and only if the transaction has been authorized by the rekeyed account. +Rekeying is an Algorand feature that enables users to transfer the authorization power of their account to a different account. When an account has been rekeyed, any future transactions from that account will be accepted by the blockchain only if they have been authorized by the rekeyed account. -A user can rekey their account to the selected account by sending a rekey-to transaction with rekey-to field set to the target account address. A rekey-to transaction is atransaction which has the rekey-to field set to a well formed Algorand address. -Any algorand account can be rekeyed by sending a rekey-to transaction from that account, this includes the contract accounts. +A user can rekey their account to a chosen account by sending a rekey-to transaction with the RekeyTo field set to the target account address. A rekey-to transaction is a transaction that has the RekeyTo field set to a well-formed Algorand address. Any Algorand account, including contract accounts, can be rekeyed by sending a rekey-to transaction from that account. -Contract accounts are accounts which are derived from the Teal code that is in control of that account. Anyone can set the fields and submit a transaction from the contract account as long as it passes the checks enforced in the Teal code. This results in an issue if the Teal code is supposed to approve a transaction that passes specific checks and does not check the rekey-to field. A malicious user can first send a transaction approved by the Teal code with rekey-to set to their account. After rekeying, the attacker can transfer the assets, algos directly by authorizing the transactions with their private key. +Contract accounts are accounts controlled by Teal code. Anyone can set fields and submit transactions from a contract account as long as they pass the checks enforced in the Teal code. If the Teal code approves a transaction that passes specific checks but does not verify the RekeyTo field, a malicious user can send a transaction approved by the Teal code with the RekeyTo field set to their account. After rekeying, the attacker can transfer assets and Algos directly by authorizing transactions with their private key. -Similar issue affects the accounts that created a delegate signature by signing a Teal program. Delegator is only needed to sign the contract and any user with access to delegate signature can construct and submit transactions. Because of this, a malicious user can rekey the sender’s account if the Teal logic accepts a transaction with the rekey-to field set to the user controlled address. +A similar issue affects accounts that have created a delegate signature by signing a Teal program. The delegator only needs to sign the contract, and any user with access to the delegate signature can construct and submit transactions. Due to this, a malicious user can rekey the sender's account if the Teal logic accepts a transaction with the RekeyTo field set to a user-controlled address. -Note: From Teal v6, Applications can also be rekeyed by executing an inner transaction with "RekeyTo" field set to a non-zero address. Rekeying an application allows to bypass the application logic and directly transfer Algos and assets of the application account. +Note: Starting from Teal v6, applications can also be rekeyed by executing an inner transaction with the "RekeyTo" field set to a non-zero address. Rekeying an application allows for bypassing the application logic and directly transferring Algos and assets of the application account. ## Exploit Scenarios -A user creates a delegate signature for recurring payments. Attacker rekeys the sender’s account by specifying the rekey-to field in a valid payment transaction. +A user creates a delegate signature for recurring payments. An attacker rekeys the sender's account by specifying the RekeyTo field in a valid payment transaction. ## Example -Note: This code contains several other vulnerabilities, [Unchecked Transaction Fees](../unchecked_transaction_fee), [Closing Account](../closing_account), [Time-based Replay Attack](../time_based_replay_attack). +Note: This code contains several other vulnerabilities, including [Unchecked Transaction Fees](../unchecked_transaction_fee), [Closing Account](../closing_account), and [Time-based Replay Attack](../time_based_replay_attack). ```py def withdraw( @@ -43,8 +42,8 @@ def withdraw( ## Recommendations -- For the Teal programs written in Teal version 2 or greater, either used as delegate signature or contract account, include a check in the program that verifies rekey-to field to be equal to ZeroAddress or any intended address. Teal contracts written in Teal version 1 are not affected by this issue. Rekeying feature is introduced in version 2 and Algorand rejects transactions that use features introduced in the versions later than the executed Teal program version. +- For Teal programs written in Teal version 2 or greater, either used as a delegate signature or contract account, include a check in the program that verifies the RekeyTo field to be equal to the ZeroAddress or any intended address. Teal contracts written in Teal version 1 are not affected by this issue. The rekeying feature was introduced in version 2, and Algorand rejects transactions that use features introduced in later versions than the executed Teal program version. - Use [Tealer](https://github.com/crytic/tealer) to detect this issue. -- For Applications, verify that user provided value is not used for `RekeyTo` field of a inner transaction. Additionally, avoid rekeying an application to admin controlled address. This allows for the possibility of "rug pull" by a malicious admin. +- For Applications, verify that user-provided values are not used for the `RekeyTo` field of an inner transaction. Additionally, avoid rekeying an application to an admin-controlled address, as this allows the possibility of a "rug pull" by a malicious admin. diff --git a/not-so-smart-contracts/algorand/time_based_replay_attack/README.md b/not-so-smart-contracts/algorand/time_based_replay_attack/README.md index 3c19b693..3eee6984 100644 --- a/not-so-smart-contracts/algorand/time_based_replay_attack/README.md +++ b/not-so-smart-contracts/algorand/time_based_replay_attack/README.md @@ -1,20 +1,20 @@ # Time-based Replay Attack -Lack of check for lease field in smart signatures that intend to approve a single transaction in the particular period allows attackers to submit multiple valid transactions in that period. +A lack of checks for the lease field in smart signatures, which are meant to approve a single transaction within a specific period, allows attackers to submit multiple valid transactions during that timeframe. ## Description -Algorand stops transaction replay attacks using a validity period. A validity period of a transaction is the sequence of blocks between FirstValid block and LastValid block. The transaction is considered valid only in that period and a transaction with the same hash can be processed only once in that period. Algorand also limits the period to a maximum of 1000 blocks. This allows the transaction creator to select the FirstValid, LastValid fields appropriately and feel assured that the transaction is processed only once in that period. +Algorand prevents transaction replay attacks by using a validity period. A transaction's validity period comprises the sequence of blocks between the FirstValid and LastValid blocks. A transaction is considered valid only within that period, and a transaction with the same hash can be processed only once during that timeframe. Furthermore, Algorand limits the validity period to a maximum of 1000 blocks. This allows the transaction creator to appropriately select the FirstValid and LastValid fields, ensuring that the transaction is processed only once within that period. -However, The same does not apply for transactions authorized by smart signatures. Even if the contract developer verifies the FirstValid and LastValid transaction fields to fixed values, an attacker can submit multiple transactions that are valid as per the contract. This is because any user can create and submit transactions authorized by a smart signature. The attacker can create transactions which have equal values for most transaction fields, for fields verified in the contract and slightly different values for the rest. Each one of these transactions will have a different hash and will be accepted by the protocol. +However, this does not apply to transactions authorized by smart signatures. Even if the contract developer sets the FirstValid and LastValid transaction fields to fixed values, an attacker can still submit multiple transactions that comply with the contract because any user can create and submit transactions authorized by a smart signature. The attacker can craft transactions with identical values for most fields—those verified by the contract—and slightly different values for the remaining fields. Each of these transactions will have a unique hash and will be accepted by the protocol. ## Exploit Scenarios -A user creates a delegate signature for recurring payments. Contract verifies the FirstValid and LastValid to only allow a single transaction each time. Attacker creates and submits multiple valid transactions with different hashes. +A user creates a delegate signature for recurring payments. The contract confirms the FirstValid and LastValid fields to permit only one transaction each time. However, an attacker can create and submit multiple valid transactions with different hashes. ## Examples -Note: This code contains several other vulnerabilities, see [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), [Closing Account](../closing_account). +Note: This code contains several other vulnerabilities; see [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), and [Closing Account](../closing_account). ```py def withdraw( @@ -36,4 +36,4 @@ def withdraw( ## Recommendations -Verify that the Lease field of the transaction is set to a specific value. Lease enforces mutual exclusion, once a transaction with non-zero lease is confirmed by the protocol, no other transactions with same lease and sender will be accepted till the LastValid block +Verify that the Lease field of the transaction is set to a specific value. Lease enforces mutual exclusion; once a transaction with a non-zero lease is confirmed by the protocol, no other transactions with the same lease and sender will be accepted until the LastValid block. diff --git a/not-so-smart-contracts/algorand/unchecked_transaction_fee/README.md b/not-so-smart-contracts/algorand/unchecked_transaction_fee/README.md index c3d3b7dc..e9204a94 100644 --- a/not-so-smart-contracts/algorand/unchecked_transaction_fee/README.md +++ b/not-so-smart-contracts/algorand/unchecked_transaction_fee/README.md @@ -1,16 +1,16 @@ # Unchecked Transaction Fee -Lack of transaction fee check in smart signatures allows malicious users to drain the contract account or the delegator’s account by specifying excessive fees. +The absence of a transaction fee check in smart signatures allows malicious users to drain a contract account or a delegator's account by specifying excessive fees. ## Description -Any user can submit transactions using the smart signatures and decide on the transaction fields. It is the responsibility of the creator to enforce restrictions on all the transaction fields to prevent malicious users from misusing the smart signature. +Any user can submit transactions using smart signatures and decide on the transaction fields. It is the creator's responsibility to enforce restrictions on all transaction fields to prevent malicious users from misusing the smart signature. -One of these transaction fields is Fee. Fee field specifies the number of micro-algos paid for processing the transaction. Protocol only verifies that the transaction pays a fee greater than protocol decided minimum fee. If a smart signature doesn’t bound the transaction fee, a user could set an excessive fee and drain the sender funds. Sender will be the signer of the Teal program in case of delegate signature and the contract account otherwise. +One of these transaction fields is the Fee. The Fee field specifies the number of micro-algos paid for processing the transaction. The protocol only verifies that the transaction pays a fee greater than the protocol-decided minimum fee. If a smart signature doesn't limit the transaction fee, a user could set an excessive fee and drain the sender's funds. The sender will be the signer of the Teal program in the case of a delegate signature, and the contract account otherwise. ## Exploit Scenarios -A user creates a delegate signature for recurring payments. Attacker creates a valid transaction and drains the user funds by specifying excessive fee. +A user creates a delegate signature for recurring payments. An attacker creates a valid transaction and drains the user's funds by specifying an excessive fee. ## Examples @@ -36,4 +36,4 @@ def withdraw( ## Recommendations -- Force the transaction fee to be `0` and use fee pooling. If the users should be able to call the smart signature outside of a group, force the transaction fee to be minimum transaction fee: `global MinTxnFee`. +- Set the transaction fee to `0` and use fee pooling. If users need to call the smart signature outside of a group, set the transaction fee to the minimum transaction fee: `global MinTxnFee`.