Skip to content

Commit 781a325

Browse files
montylymontyly
authored
Merge pull request #385 from crytic/ton
TON not so smart contracts
2 parents 2d638c7 + 7b18e13 commit 781a325

File tree

6 files changed

+241
-0
lines changed

6 files changed

+241
-0
lines changed

Diff for: ‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ Brought to you by [Trail of Bits](https://www.trailofbits.com/), this repository
2727
- [Cosmos](./not-so-smart-contracts/cosmos)
2828
- [Substrate](./not-so-smart-contracts/substrate)
2929
- [Solana](./not-so-smart-contracts/solana)
30+
- [TON](./not-so-smart-contracts/ton)
3031
- [Program Analysis](./program-analysis): Using automated tools to secure contracts
3132
- [Echidna](./program-analysis/echidna): A fuzzer that checks your contract's properties
3233
- [Medusa](./program-analysis/medusa/docs/src): A next-gen fuzzer that checks your contract's properties

Diff for: ‎not-so-smart-contracts/README.md

+1
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ This repository contains examples of common smart contract vulnerabilities, incl
77
- [Cosmos](./cosmos/README.md)
88
- [Solana](./solana/README.md)
99
- [Substrate](./substrate/README.md)
10+
- [TON](./ton/README.md)

Diff for: ‎not-so-smart-contracts/ton/README.md

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# (Not So) Smart Contracts
2+
3+
This repository contains examples of common TON smart contract vulnerabilities, featuring code from real smart contracts. Utilize the Not So Smart Contracts to learn about TON vulnerabilities, refer to them during security reviews, and use them as a benchmark for security analysis tools.
4+
5+
## Features
6+
7+
Each _Not So Smart Contract_ consists of a standard set of information:
8+
9+
- Vulnerability type description
10+
- Attack scenarios to exploit the vulnerability
11+
- Recommendations to eliminate or mitigate the vulnerability
12+
- Real-world contracts exhibiting the flaw
13+
- References to third-party resources providing more information
14+
15+
## Vulnerabilities
16+
17+
| Not So Smart Contract | Description |
18+
| -------------------------------------------------------------- | ----------------------------------------------------------- |
19+
| [Int as boolean](int_as_boolean) | Unexpected result of logical operations on the int type |
20+
| [Fake Jetton contract](fake_jetton_contract) | Any contract can send a `transfer_notification` message |
21+
| [Forward TON without gas check](forward_ton_without_gas_check) | Users can drain TON balance of a contract lacking gas check |
22+
23+
## Credits
24+
25+
These examples are developed and maintained by [Trail of Bits](https://www.trailofbits.com/).
26+
27+
If you have any questions, issues, or wish to learn more, join the #ethereum channel on the [Empire Hacking Slack](https://slack.empirehacking.nyc/) or [contact us](https://www.trailofbits.com/contact/) directly.

Diff for: ‎not-so-smart-contracts/ton/fake_jetton_contract/README.md

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# Fake Jetton Contract
2+
3+
TON smart contracts use the `transfer_notification` message sent by the receiver's Jetton wallet contract to specify and process a user request along with the transfer of a Jetton. Users add a `forward_payload` to the Jetton `transfer` message when transferring their Jettons, this `forward_payload` is forwarded by the receiver's Jetton wallet contract to the receiver in the `transfer_notification` message. The `transfer_notification` message has the following TL-B schema:
4+
5+
```
6+
transfer_notification#7362d09c query_id:uint64 amount:(VarUInteger 16)
7+
sender:MsgAddress forward_payload:(Either Cell ^Cell)
8+
= InternalMsgBody;
9+
```
10+
11+
The `amount` and `sender` are added by the receiver's Jetton wallet contract as the amount of Jettons transferred and the sender of Jettons (owner of the Jetton wallet that sent of the `internal_transfer` message). However, all the other values specified by the user in the `forward_payload` are not parsed or validated by the Jetton wallet contract, they are added as it and sent in the `transfer_notification` message. Therefore, the receiver of the `transfer_notification` message must consider malicious values in the `forward_payload` and validate them properly to prevent any contract state manipulation.
12+
13+
## Example
14+
15+
The following simplified code highlights the lack of token_id validation in the `transfer_notification` message. This contract tracks user deposits by updating the `token0_balances` dictionary entry for the user's address. However, the `transfer_notification` message handler does not verify that the `sender_address` is equal to one of the `token0` or `token1` Jetton wallets owned by this contract. This allows users to manipulate their deposit values by sending the `transfer_notification` message from a fake Jetton wallet contract.
16+
17+
```FunC
18+
#include "imports/stdlib.fc";
19+
20+
() recv_internal(int my_balance, int msg_value, cell in_msg_full, slice in_msg_body) impure {
21+
slice cs = in_msg_full.begin_parse();
22+
int flags = cs~load_uint(4);
23+
;; ignore all bounced messages
24+
if (is_bounced?(flags)) {
25+
return ();
26+
}
27+
slice sender_address = cs~load_msg_addr(); ;; incorrectly assumed to be Jetton wallet contract owned by this contract
28+
29+
(cell token0_balances, cell token1_balances) = load_data(); ;; balances dictionaries
30+
31+
(int op, int query_id) = in_msg_body~load_op_and_query_id();
32+
33+
if (op == op::transfer_notification) {
34+
(int amount, slice from_address) = (in_msg_body~load_coins(), in_msg_body~load_msg_addr());
35+
cell forward_payload_ref = in_msg_body~load_ref();
36+
slice forward_payload = forward_payload_ref.begin_parse();
37+
38+
int is_token0? = forward_payload~load_int(1);
39+
40+
if (is_token0?) {
41+
slice balance_before = token0_balances.dict_get?(267, from_address);
42+
int balance = balance_before~load_coins();
43+
balance = balance + amount;
44+
slice balance_after = begin_cell().store_coinds(balance).end_cell().being_parse();
45+
token0_balances~dict_set(267, from_address, balance_after);
46+
} else {
47+
slice balance_before = token1_balances.dict_get?(267, from_address);
48+
int balance = balance_before~load_coins();
49+
balance = balance + amount;
50+
slice balance_after = begin_cell().store_coinds(balance).end_cell().being_parse();
51+
token1_balances~dict_set(267, from_address, balance_after);
52+
}
53+
54+
save_data();
55+
return ();
56+
}
57+
}
58+
```
59+
60+
## Mitigations
61+
62+
- Store the address of Jetton wallet contract owned by the current contract at the time of contract initialization and use this stored value to verify the sender of the `transfer_notification` message.
63+
- Validate all the user provided values in the `forward_payload` instead of trusting users to send correct values.

Diff for: ‎not-so-smart-contracts/ton/forward_ton_without_gas_check/README.md

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
# Foward TON without gas check
2+
3+
TON smart contracts needs to send TON as gas fee along with the Jetton transfers or any other message they send to another contract. If a contract allows its users to specify the amount of TON to be sent with an outgoing message then it must check that the user supplied enough TON with their message to cover for the transaction fee and the forward TON amount.
4+
5+
If a contract lacks such a gas check then users can specify a higher forward TON amount to drain the TON balance of the smart contract, freezing the smart contract and potentially destrying it.
6+
7+
## Example
8+
9+
The following simplified code highlights the lack of gas check. The following contract implements a `withdraw` operation that allows users to specify a forward TON amount to and forward payload to send with the Jettons. However, this contract does not chekc if used included enough TON with the `withdraw` message to cover for the `withdraw` message transaction, jetton transfer gas fee, and forward TON amount. This allows users to drain the TON balance of the smart contract.
10+
11+
```FunC
12+
#include "imports/stdlib.fc";
13+
14+
() recv_internal(int my_balance, int msg_value, cell in_msg_full, slice in_msg_body) impure {
15+
slice cs = in_msg_full.begin_parse();
16+
int flags = cs~load_uint(4);
17+
;; ignore all bounced messages
18+
if (is_bounced?(flags)) {
19+
return ();
20+
}
21+
slice sender_address = cs~load_msg_addr(); ;; incorrectly assumed to be Jetton wallet contract owned by this contract
22+
23+
(int op, int query_id) = in_msg_body~load_op_and_query_id();
24+
25+
if (op == op::withdraw) {
26+
int amount = in_msg_body~load_coins();
27+
slice to_address = in_msg_body~load_msg_addr();
28+
int forward_ton_amount = in_msg_body~load_coins(); ;; user specified forward TON amount
29+
cell forward_payload = begin_cell().store_slice(in_msg_body).end_cell();
30+
31+
var msg_body = begin_cell()
32+
.store_op_and_query_id(op::transfer, query_id)
33+
.store_coins(amount)
34+
.store_slice(to_address)
35+
.store_slice(to_address)
36+
.store_uint(0, 1)
37+
.store_coins(forward_ton_amount)
38+
.store_maybe_ref(forward_payload)
39+
.end_cell();
40+
41+
cell msg = begin_cell()
42+
.store_uint(0x18, 6)
43+
.store_slice(USDT_WALLET_ADDRESS)
44+
.store_coins(JETTON_TRANSFER_GAS + forward_ton_amount) ;; sending user specified forward TON amount
45+
.store_uint(1, 1 + 4 + 4 + 64 + 32 + 1 + 1) ;; message parameters
46+
.store_ref(ref)
47+
.end_cell();
48+
49+
send_raw_message(msg, 0);
50+
51+
return ();
52+
}
53+
}
54+
```
55+
56+
## Mitigations
57+
58+
- Avoid allowing users to specify forward TON amount with the outgoing messages.
59+
- Always check that the user sent enough TON in the `msg_value` to cover for the current transaction fee and sum of all the outgoing message values.

Diff for: ‎not-so-smart-contracts/ton/int_as_boolean/README.md

+90
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
# Using int as boolean values
2+
3+
In FunC, booleans are represented as integers; false is represented as 0 and true is represented as -1 (257 ones in binary notation).
4+
5+
Logical operations are done as bitwise operations over the binary representation of the integer values. Notably, The not operation `~` flips all the bits of an integer value; therefore, a non-zero value other than -1 becomes another non-zero value.
6+
7+
When a condition is checked, every non-zero integer is considered a true value. This, combined with the logical operations being bitwise operations, leads to an unexpected behavior of `if` conditions using the logical operations.
8+
9+
## Example
10+
11+
The following simplified code highlights the unexpected behavior of the `~` operator on a non-zero interger value.
12+
13+
```FunC
14+
#include "imports/stdlib.fc";
15+
16+
() recv_internal(int my_balance, int msg_value, cell in_msg_full, slice in_msg_body) impure {
17+
int correct_true = -1;
18+
if (correct_true) {
19+
~strdump("correct_true is true"); ;; printed
20+
} else {
21+
~strdump("correct_true is false");
22+
}
23+
24+
if (~ correct_true) {
25+
~strdump("~correct_true is true");
26+
} else {
27+
~strdump("~correct_true is false"); ;; printed
28+
}
29+
30+
int correct_false = 0;
31+
if (correct_false) {
32+
~strdump("correct_false is true");
33+
} else {
34+
~strdump("correct_false is false"); ;; printed
35+
}
36+
37+
if (~ correct_false) {
38+
~strdump("~correct_false is true"); ;; printed
39+
} else {
40+
~strdump("~correct_false is false");
41+
}
42+
43+
int positive = 10;
44+
if (positive) {
45+
~strdump("positive is true"); ;; printed
46+
} else {
47+
~strdump("positive is false");
48+
}
49+
50+
if (~ positive) {
51+
~strdump("~positive is true"); ;; printed but unexpected
52+
} else {
53+
~strdump("~positive is false");
54+
}
55+
56+
int negative = -10;
57+
if (negative) {
58+
~strdump("negative is true"); ;; printed
59+
} else {
60+
~strdump("negative is false");
61+
}
62+
63+
if (~ negative) {
64+
~strdump("~negative is true"); ;; printed but unexpected
65+
} else {
66+
~strdump("~negative is false");
67+
}
68+
}
69+
```
70+
71+
The `recv_internal` function above prints the following debug logs:
72+
73+
```
74+
#DEBUG#: correct_true is true
75+
#DEBUG#: ~correct_true is false
76+
#DEBUG#: correct_false is false
77+
#DEBUG#: ~correct_false is true
78+
#DEBUG#: positive is true
79+
#DEBUG#: ~positive is true
80+
#DEBUG#: negative is true
81+
#DEBUG#: ~negative is true
82+
```
83+
84+
It demonstrats that the `~ 10` and `~ -10` both evaluate to `true` instead of becoming `false` with the `~` operator.
85+
86+
## Mitigations
87+
88+
- Always use `0` or `-1` in condition checks to get correct results.
89+
- Be careful with the logical operator usage on non-zero integer values.
90+
- Implement test cases to verify correct behavior of all condition checks with different interger values.

0 commit comments

Comments
 (0)