Skip to content

Commit 753ba42

Browse files
committed
Merge branch 'master' into release
2 parents 178998a + 7154050 commit 753ba42

File tree

28 files changed

+1042
-29
lines changed

28 files changed

+1042
-29
lines changed

.github/dependabot.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
version: 2
2+
updates:
3+
- package-ecosystem: "github-actions"
4+
directory: "/"
5+
schedule:
6+
interval: "daily"

.github/workflows/ci.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,17 @@ on:
1212

1313
jobs:
1414
tests:
15-
runs-on: ubuntu-18.04
15+
runs-on: ubuntu-22.04
1616
strategy:
1717
fail-fast: false
1818
matrix:
1919
type: ["slither", "manticore"]
2020
steps:
21-
- uses: actions/checkout@v1
22-
- name: Set up Python 3.6
23-
uses: actions/setup-python@v1
21+
- uses: actions/checkout@v3
22+
- name: Set up Python 3.8
23+
uses: actions/setup-python@v4
2424
with:
25-
python-version: 3.6
25+
python-version: 3.8
2626
- name: Install dependencies
2727
run: |
2828
sudo wget -O /usr/bin/solc https://github.com/ethereum/solidity/releases/download/v0.5.11/solc-static-linux

.github/workflows/echidna.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jobs:
1414
tests:
1515
name: ${{ matrix.name }}
1616
continue-on-error: ${{ matrix.flaky == true }}
17-
runs-on: ubuntu-18.04
17+
runs-on: ubuntu-22.04
1818
strategy:
1919
fail-fast: false
2020
matrix:

development-guidelines/guidelines.md

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,20 @@
22

33
Follow these high-level recommendations to build more secure smart contracts.
44

5-
* [Design (before development)](#design-guidelines)
6-
* [Documentation and specifications](#documentation-and-specifications)
7-
* [On-chain vs off-chain computation](#on-chain-vs-off-chain-computation)
8-
* [Upgradeability](#upgradeability)
9-
* [Implementation (during development)](#implementation-guidelines)
10-
* [Function composition](#function-composition)
11-
* [Inheritance](#inheritance)
12-
* [Events](#events)
13-
* [Avoid known pitfalls](#avoid-known-pitfalls)
14-
* [Dependencies](#dependencies)
15-
* [Testing and verification](#testing-and-verification)
16-
* [Solidity](#solidity)
17-
* [Deploymnent (after development)](#deployment-guidelines)
5+
- [Development Guidelines](#development-guidelines)
6+
- [Design guidelines](#design-guidelines)
7+
- [Documentation and specifications](#documentation-and-specifications)
8+
- [On-chain vs off-chain computation](#on-chain-vs-off-chain-computation)
9+
- [Upgradeability](#upgradeability)
10+
- [Implementation guidelines](#implementation-guidelines)
11+
- [Function composition](#function-composition)
12+
- [Inheritance](#inheritance)
13+
- [Events](#events)
14+
- [Avoid known pitfalls](#avoid-known-pitfalls)
15+
- [Dependencies](#dependencies)
16+
- [Testing and verification](#testing-and-verification)
17+
- [Solidity](#solidity)
18+
- [Deployment guidelines](#deployment-guidelines)
1819

1920
## Design guidelines
2021

@@ -76,13 +77,12 @@ The architecture of your codebase should make your code easy to review. Avoid ar
7677
### Testing and verification
7778

7879
- **Write thorough unit-tests.** An extensive test suite is crucial to build high-quality software.
79-
- **Write [Slither](https://github.com/crytic/slither), [Echidna](https://github.com/crytic/echidna) and [Manticore](https://github.com/trailofbits/manticore) custom checks and properties.** Automated tools will help ensure your contract is secure. Review the rest of this guide to learn how to write efficient checks and properties.
80-
- **Use [crytic.io](https://crytic.io/).** Crytic integrates with Github, provides access to private Slither detectors, and runs custom property checks from Echidna.
80+
- **Write [Slither](https://github.com/crytic/slither) and [Echidna](https://github.com/crytic/echidna) custom checks and properties.** Automated tools will help ensure your contract is secure. Review the rest of this guide to learn how to write efficient checks and properties.
8181

8282
### Solidity
8383

84-
- **Favor Solidity 0.5 or 0.6.** In our opinion, Solidity 0.5 and 0.6 are more secure and have better built-in practices than 0.4. Solidity 0.7 is too young to be used in production and needs time to mature.
85-
- **Use a stable release to compile; use the latest release to check for warnings.** Check that your code has no reported issues with the latest compiler version. However, Solidity has a fast release cycle and has a history of compiler bugs, so we do not recommend the latest version for deployment (see Slither’s [solc version recommendation](https://github.com/crytic/slither/wiki/Detector-Documentation#recommendation-39)).
84+
- **Favor Solidity versions outlined in our [Slither Recommendations](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity)** In our opinion, older Solidity are more secure and have better built-in practices. Newer Solidity versions may be ttoo young to be used in production and require additional time to mature.
85+
- **Use a stable release to compile; use the latest release to check for warnings.** Check that your code has no reported issues with the latest compiler version. However, Solidity has a fast release cycle and has a history of compiler bugs, so we do not recommend the latest version for deployment (see Slither’s [solc version recommendation](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity)).
8686
- **Do not use inline assembly.** Assembly requires EVM expertise. Do not write EVM code if you have not _mastered_ the yellow paper.
8787

8888
## Deployment guidelines

development-guidelines/incident_response.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,13 @@ Here, we provide recommendations around the formulation of an incident response
1818
- Consider adding more resilient solutions for detection and mitigation, especially in terms of specific alternate endpoints and queries for different data as well as status pages and support contacts for affected services.
1919
- [ ] **Combine issues and determine whether new detection and mitigation scenarios are needed.**
2020
- [ ] **Perform periodic dry runs of specific scenarios in the incident response plan to find gaps and opportunities for improvement and to develop muscle memory.**
21-
- Document the intervals at which the team should perform dry runs of the various scenarios. For scenarios that are more likely to happen, perform dry runs more regularly. Create a template to be filled in after a dry run to describe the improvements that need to be made to the incident response.
21+
- Document the intervals at which the team should perform dry runs of the various scenarios. For scenarios that are more likely to happen, perform dry runs more regularly. Create a template to be filled in after a dry run to describe the improvements that need to be made to the incident response.
22+
23+
## Incident Response Plan Resources
24+
25+
- [How to Hack the Yield Protocol](https://docs.yieldprotocol.com/#/operations/how_to_hack)
26+
- [Emergency Steps – Yearn](https://github.com/yearn/yearn-devdocs/blob/master/docs/developers/v2/EMERGENCY.md)
27+
28+
## Well-handled IR Incidents
29+
30+
- [Yield Protocol](https://medium.com/yield-protocol/post-mortem-of-incident-on-august-5th-2022-7bb70dbb9ada)

development-guidelines/workflow.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ Finally, be mindful of issues that automated tools cannot easily find:
3434

3535
## Ask for help
3636

37-
[Ethereum office hours](https://calendly.com/dan-trailofbits/office-hours) run every Tuesday afternoon. These 1-hour, 1-on-1 sessions are an opportunity to ask us any questions you have about security, troubleshoot using our tools, and get feedback from experts about your current approach. We will help you work through this guide.
37+
[Office Hours](https://calendly.com/trail-of-bits-sales/trail-of-bits-office-hours) run every Tuesday afternoon. These 1-hour, 1-on-1 sessions are an opportunity to ask us any questions you have about security, troubleshoot using our tools, and get feedback from experts about your current approach. We will help you work through this guide.
3838

3939
Join our Slack: [Empire Hacking](https://join.slack.com/t/empirehacking/shared_invite/zt-h97bbrj8-1jwuiU33nnzg67JcvIciUw). We're always available in the #crytic and #ethereum channels if you have any questions.
4040

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# (Not So) Smart Contracts
2+
3+
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.
4+
5+
## Features
6+
7+
Each _Not So Smart Contract_ includes a standard set of information:
8+
9+
* Description of the vulnerability type
10+
* Attack scenarios to exploit the vulnerability
11+
* Recommendations to eliminate or mitigate the vulnerability
12+
* Real-world contracts that exhibit the flaw
13+
* References to third-party resources with more information
14+
15+
## Vulnerabilities
16+
17+
| Not So Smart Contract | Description |
18+
| --- | --- |
19+
| [Rekeying](rekeying) | Smart signatures are rekeyable |
20+
| [Unchecked Transaction Fees](unchecked_transaction_fee) | Attacker sets excessive fees for smart signature transactions |
21+
| [Closing Account](closing_account) | Attacker closes smart signature accounts |
22+
| [Closing Asset](closing_asset) | Attacker transfers entire asset balance of a smart signature |
23+
| [Group Size Check](group_size_check) | Contract does not check transaction group size |
24+
| [Time-based Replay Attack](time_based_replay_attack) | Contract does not use lease for periodic payments |
25+
| [Access Controls](access_controls) | Contract does not enfore access controls for updating and deleting application |
26+
| [Asset Id Check](asset_id_check) | Contract does not check asset id for asset transfer operations |
27+
| [Denial of Service](denial_of_service) | Attacker stalls contract execution by opting out of a asset |
28+
29+
## Credits
30+
31+
These examples are developed and maintained by [Trail of Bits](https://www.trailofbits.com/).
32+
33+
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.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# Access Controls
2+
3+
Lack of appropriate checks for application calls of type UpdateApplication and DeleteApplication allows attackers to update application’s code or delete an application entirely.
4+
5+
## Description
6+
7+
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.
8+
This allows attackers to update or delete the application if proper access controls are not enforced in the application.
9+
10+
## Exploit Scenarios
11+
12+
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.
13+
14+
## Recommendations
15+
16+
- Set proper access controls and apply various checks before approving applications calls of type UpdateApplication and DeleteApplication.
17+
18+
- Use [Tealer](https://github.com/crytic/tealer) to detect this issue.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# Asset Id Check
2+
3+
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.
4+
5+
## Description
6+
7+
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.
8+
9+
## Exploit Scenarios
10+
11+
- 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.
12+
- User creates a delegate signature that allows recurring transfers of a certain asset. Attacker creates a valid asset transfer transaction of more valuable assets.
13+
14+
## Examples
15+
16+
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).
17+
18+
```py
19+
def withdraw_asset(
20+
duration,
21+
period,
22+
amount,
23+
receiver,
24+
timeout,
25+
):
26+
return And(
27+
Txn.type_enum() == TxnType.AssetTransfer,
28+
Txn.first_valid() % period == Int(0),
29+
Txn.last_valid() == Txn.first_valid() + duration,
30+
Txn.asset_receiver() == receiver,
31+
Txn.asset_amount() == amount,
32+
Txn.first_valid() < timeout,
33+
)
34+
```
35+
36+
## Recommendations
37+
38+
Verify the asset id to be expected asset for all asset related operations in the contract.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Closing Account
2+
3+
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.
4+
5+
## Description
6+
7+
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.
8+
9+
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.
10+
11+
## Exploit Scenarios
12+
13+
A user creates a delegate signature for recurring payments. Attacker creates a valid transaction and sets the CloseRemainderTo field to their address.
14+
15+
## Examples
16+
17+
Note: This code contains several other vulnerabilities, see [Rekeying](../rekeying), [Unchecked Transaction Fees](../unchecked_transaction_fee), [Time-based Replay Attack](../time_based_replay_attack).
18+
19+
```py
20+
def withdraw(
21+
duration,
22+
period,
23+
amount,
24+
receiver,
25+
timeout,
26+
):
27+
return And(
28+
Txn.type_enum() == TxnType.Payment,
29+
Txn.first_valid() % period == Int(0),
30+
Txn.last_valid() == Txn.first_valid() + duration,
31+
Txn.receiver() == receiver,
32+
Txn.amount() == amount,
33+
Txn.first_valid() < timeout,
34+
)
35+
```
36+
37+
## Recommendations
38+
39+
Verify that the CloseRemainderTo field is set to the ZeroAddress or to any intended address before approving the transaction in the Teal contract.

0 commit comments

Comments
 (0)