From 9b477fe4a5725a8c689d449a4e1a268ea11eb59f Mon Sep 17 00:00:00 2001 From: Tarun Bansal Date: Fri, 24 Jan 2025 11:58:44 +0530 Subject: [PATCH 1/5] Add TON not so smart contracts examples. * Integer as Boolean values --- not-so-smart-contracts/README.md | 1 + not-so-smart-contracts/ton/README.md | 25 ++++++ .../ton/int_as_boolean/README.md | 89 +++++++++++++++++++ 3 files changed, 115 insertions(+) create mode 100644 not-so-smart-contracts/ton/README.md create mode 100644 not-so-smart-contracts/ton/int_as_boolean/README.md diff --git a/not-so-smart-contracts/README.md b/not-so-smart-contracts/README.md index c70b6c16..f88ad038 100644 --- a/not-so-smart-contracts/README.md +++ b/not-so-smart-contracts/README.md @@ -7,3 +7,4 @@ This repository contains examples of common smart contract vulnerabilities, incl - [Cosmos](./cosmos/README.md) - [Solana](./solana/README.md) - [Substrate](./substrate/README.md) +- [TON](./ton/README.md) diff --git a/not-so-smart-contracts/ton/README.md b/not-so-smart-contracts/ton/README.md new file mode 100644 index 00000000..e07d7e0b --- /dev/null +++ b/not-so-smart-contracts/ton/README.md @@ -0,0 +1,25 @@ +# (Not So) Smart Contracts + +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. + +## Features + +Each _Not So Smart Contract_ consists of a standard set of information: + +- Vulnerability type description +- Attack scenarios to exploit the vulnerability +- Recommendations to eliminate or mitigate the vulnerability +- Real-world contracts exhibiting the flaw +- References to third-party resources providing more information + +## Vulnerabilities + +| Not So Smart Contract | Description | +| ---------------------------------------------------------------------------- | ------------------------------------------------------------ | +| [Int as Boolean](int_as_boolean) | Unexpected result of logical operations on the int type | + +## Credits + +These examples are developed and maintained by [Trail of Bits](https://www.trailofbits.com/). + +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 --git a/not-so-smart-contracts/ton/int_as_boolean/README.md b/not-so-smart-contracts/ton/int_as_boolean/README.md new file mode 100644 index 00000000..83403fd2 --- /dev/null +++ b/not-so-smart-contracts/ton/int_as_boolean/README.md @@ -0,0 +1,89 @@ +# Using int as boolean values + +In FunC, booleans are represented as integers; false is represented as 0 and true is represented as -1 (257 ones in binary notation). + +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. + +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. + +## Example + +The following simplified code highlights the unexpected behavior of the `~` operator on a non-zero interger value. + +```FunC +#include "imports/stdlib.fc"; + +() recv_internal(int my_balance, int msg_value, cell in_msg_full, slice in_msg_body) impure { + int correct_true = -1; + if (correct_true) { + ~strdump("correct_true is true"); ;; printed + } else { + ~strdump("correct_true is false"); + } + + if (~ correct_true) { + ~strdump("~correct_true is true"); + } else { + ~strdump("~correct_true is false"); ;; printed + } + + int correct_false = 0; + if (correct_false) { + ~strdump("correct_false is true"); + } else { + ~strdump("correct_false is false"); ;; printed + } + + if (~ correct_false) { + ~strdump("~correct_false is true"); ;; printed + } else { + ~strdump("~correct_false is false"); + } + + int positive = 10; + if (positive) { + ~strdump("positive is true"); ;; printed + } else { + ~strdump("positive is false"); + } + + if (~ positive) { + ~strdump("~positive is true"); ;; printed but unexpected + } else { + ~strdump("~positive is false"); + } + + int negative = -10; + if (negative) { + ~strdump("negative is true"); ;; printed + } else { + ~strdump("negative is false"); + } + + if (~ negative) { + ~strdump("~negative is true"); ;; printed but unexpected + } else { + ~strdump("~negative is false"); + } +} +``` + +The `recv_internal` function above prints the following debug logs: +``` + #DEBUG#: correct_true is true + #DEBUG#: ~correct_true is false + #DEBUG#: correct_false is false + #DEBUG#: ~correct_false is true + #DEBUG#: positive is true + #DEBUG#: ~positive is true + #DEBUG#: negative is true + #DEBUG#: ~negative is true +``` + +It demonstrats that the `~ 10` and `~ -10` both evaluate to `true` instead of becoming `false` with the `~` operator. + +## Mitigations + +- Always use `0` or `-1` in condition checks to get correct results. +- Be careful with the logical operator usage on non-zero integer values. +- Implement test cases to verify correct behavior of all condition checks with different interger values. From 3b56d690f2705fd89a80fa17b1e9905e60049fe6 Mon Sep 17 00:00:00 2001 From: Tarun Bansal Date: Fri, 24 Jan 2025 12:55:44 +0530 Subject: [PATCH 2/5] Add fake jetton contract in the TON not so smart contracts list --- README.md | 1 + not-so-smart-contracts/ton/README.md | 1 + .../ton/fake_jetton_contract/README.md | 61 +++++++++++++++++++ 3 files changed, 63 insertions(+) create mode 100644 not-so-smart-contracts/ton/fake_jetton_contract/README.md diff --git a/README.md b/README.md index e7bd2c49..0634fad3 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,7 @@ Brought to you by [Trail of Bits](https://www.trailofbits.com/), this repository - [Cosmos](./not-so-smart-contracts/cosmos) - [Substrate](./not-so-smart-contracts/substrate) - [Solana](./not-so-smart-contracts/solana) + - [TON](./not-so-smart-contracts/ton) - [Program Analysis](./program-analysis): Using automated tools to secure contracts - [Echidna](./program-analysis/echidna): A fuzzer that checks your contract's properties - [Medusa](./program-analysis/medusa/docs/src): A next-gen fuzzer that checks your contract's properties diff --git a/not-so-smart-contracts/ton/README.md b/not-so-smart-contracts/ton/README.md index e07d7e0b..94056f60 100644 --- a/not-so-smart-contracts/ton/README.md +++ b/not-so-smart-contracts/ton/README.md @@ -17,6 +17,7 @@ Each _Not So Smart Contract_ consists of a standard set of information: | Not So Smart Contract | Description | | ---------------------------------------------------------------------------- | ------------------------------------------------------------ | | [Int as Boolean](int_as_boolean) | Unexpected result of logical operations on the int type | +| [Fake Jetton contract](fake_jetton_contract) | Any contract can send a `transfer_notification` message | ## Credits diff --git a/not-so-smart-contracts/ton/fake_jetton_contract/README.md b/not-so-smart-contracts/ton/fake_jetton_contract/README.md new file mode 100644 index 00000000..5bf302a6 --- /dev/null +++ b/not-so-smart-contracts/ton/fake_jetton_contract/README.md @@ -0,0 +1,61 @@ +# Fake Jetton Contract + +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: + +``` +transfer_notification#7362d09c query_id:uint64 amount:(VarUInteger 16) + sender:MsgAddress forward_payload:(Either Cell ^Cell) + = InternalMsgBody; +``` + +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. + +## Example + +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. + +```FunC +#include "imports/stdlib.fc"; + +() recv_internal(int my_balance, int msg_value, cell in_msg_full, slice in_msg_body) impure { + slice cs = in_msg_full.begin_parse(); + int flags = cs~load_uint(4); + ;; ignore all bounced messages + if (is_bounced?(flags)) { + return (); + } + slice sender_address = cs~load_msg_addr(); ;; incorrectly assumed to be Jetton wallet contract owned by this contract + + (cell token0_balances, cell token1_balances) = load_data(); ;; balances dictionaries + + (int op, int query_id) = in_msg_body~load_op_and_query_id(); + + if (op == op::transfer_notification) { + (int amount, slice from_address) = (in_msg_body~load_coins(), in_msg_body~load_msg_addr()); + cell forward_payload_ref = in_msg_body~load_ref(); + slice forward_payload = forward_payload_ref.begin_parse(); + + int is_token0? = forward_payload.load_int(1); + + if (is_token0?) { + slice balance_before = token0_balances.dict_get?(267, from_address); + int balance = balance_before.load_coins(); + balance = balance + amount; + token0_balances~dict_set(267, from_address, balance); + } else { + slice balance_before = token1_balances.dict_get?(267, from_address); + int balance = balance_before.load_coins(); + balance = balance + amount; + token1_balances~dict_set(267, from_address, balance); + } + + save_data(); + return (); + } +} +``` + +## Mitigations + +- 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. +- Validate all the user provided values in the `forward_payload` instead of trusting users to send correct values. From 2a1473726dfb0d04ce5af2f7fe848308e346c320 Mon Sep 17 00:00:00 2001 From: Tarun Bansal Date: Fri, 24 Jan 2025 13:01:28 +0530 Subject: [PATCH 3/5] Fix fake jetton contract example --- not-so-smart-contracts/ton/fake_jetton_contract/README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/not-so-smart-contracts/ton/fake_jetton_contract/README.md b/not-so-smart-contracts/ton/fake_jetton_contract/README.md index 5bf302a6..e31901fe 100644 --- a/not-so-smart-contracts/ton/fake_jetton_contract/README.md +++ b/not-so-smart-contracts/ton/fake_jetton_contract/README.md @@ -41,12 +41,14 @@ The following simplified code highlights the lack of token_id validation in the slice balance_before = token0_balances.dict_get?(267, from_address); int balance = balance_before.load_coins(); balance = balance + amount; - token0_balances~dict_set(267, from_address, balance); + slice balance_after = begin_cell().store_coinds(balance).end_cell().being_parse(); + token0_balances~dict_set(267, from_address, balance_after); } else { slice balance_before = token1_balances.dict_get?(267, from_address); int balance = balance_before.load_coins(); balance = balance + amount; - token1_balances~dict_set(267, from_address, balance); + slice balance_after = begin_cell().store_coinds(balance).end_cell().being_parse(); + token1_balances~dict_set(267, from_address, balance_after); } save_data(); From 76685e111ca6be62b6f643a63a43133d70c653a4 Mon Sep 17 00:00:00 2001 From: Tarun Bansal Date: Fri, 24 Jan 2025 13:40:00 +0530 Subject: [PATCH 4/5] Fix formating issues --- not-so-smart-contracts/ton/README.md | 8 ++++---- not-so-smart-contracts/ton/fake_jetton_contract/README.md | 6 +++--- not-so-smart-contracts/ton/int_as_boolean/README.md | 5 +++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/not-so-smart-contracts/ton/README.md b/not-so-smart-contracts/ton/README.md index 94056f60..8a21e9ca 100644 --- a/not-so-smart-contracts/ton/README.md +++ b/not-so-smart-contracts/ton/README.md @@ -14,10 +14,10 @@ Each _Not So Smart Contract_ consists of a standard set of information: ## Vulnerabilities -| Not So Smart Contract | Description | -| ---------------------------------------------------------------------------- | ------------------------------------------------------------ | -| [Int as Boolean](int_as_boolean) | Unexpected result of logical operations on the int type | -| [Fake Jetton contract](fake_jetton_contract) | Any contract can send a `transfer_notification` message | +| Not So Smart Contract | Description | +| -------------------------------------------- | ------------------------------------------------------- | +| [Int as Boolean](int_as_boolean) | Unexpected result of logical operations on the int type | +| [Fake Jetton contract](fake_jetton_contract) | Any contract can send a `transfer_notification` message | ## Credits diff --git a/not-so-smart-contracts/ton/fake_jetton_contract/README.md b/not-so-smart-contracts/ton/fake_jetton_contract/README.md index e31901fe..3ef1798b 100644 --- a/not-so-smart-contracts/ton/fake_jetton_contract/README.md +++ b/not-so-smart-contracts/ton/fake_jetton_contract/README.md @@ -3,7 +3,7 @@ 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: ``` -transfer_notification#7362d09c query_id:uint64 amount:(VarUInteger 16) +transfer_notification#7362d09c query_id:uint64 amount:(VarUInteger 16) sender:MsgAddress forward_payload:(Either Cell ^Cell) = InternalMsgBody; ``` @@ -27,14 +27,14 @@ The following simplified code highlights the lack of token_id validation in the slice sender_address = cs~load_msg_addr(); ;; incorrectly assumed to be Jetton wallet contract owned by this contract (cell token0_balances, cell token1_balances) = load_data(); ;; balances dictionaries - + (int op, int query_id) = in_msg_body~load_op_and_query_id(); if (op == op::transfer_notification) { (int amount, slice from_address) = (in_msg_body~load_coins(), in_msg_body~load_msg_addr()); cell forward_payload_ref = in_msg_body~load_ref(); slice forward_payload = forward_payload_ref.begin_parse(); - + int is_token0? = forward_payload.load_int(1); if (is_token0?) { diff --git a/not-so-smart-contracts/ton/int_as_boolean/README.md b/not-so-smart-contracts/ton/int_as_boolean/README.md index 83403fd2..8707226a 100644 --- a/not-so-smart-contracts/ton/int_as_boolean/README.md +++ b/not-so-smart-contracts/ton/int_as_boolean/README.md @@ -1,6 +1,6 @@ # Using int as boolean values -In FunC, booleans are represented as integers; false is represented as 0 and true is represented as -1 (257 ones in binary notation). +In FunC, booleans are represented as integers; false is represented as 0 and true is represented as -1 (257 ones in binary notation). 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. @@ -69,6 +69,7 @@ The following simplified code highlights the unexpected behavior of the `~` oper ``` The `recv_internal` function above prints the following debug logs: + ``` #DEBUG#: correct_true is true #DEBUG#: ~correct_true is false @@ -80,7 +81,7 @@ The `recv_internal` function above prints the following debug logs: #DEBUG#: ~negative is true ``` -It demonstrats that the `~ 10` and `~ -10` both evaluate to `true` instead of becoming `false` with the `~` operator. +It demonstrats that the `~ 10` and `~ -10` both evaluate to `true` instead of becoming `false` with the `~` operator. ## Mitigations From 7b18e132b946583b000ff50f0d9df817e307de7a Mon Sep 17 00:00:00 2001 From: Tarun Bansal Date: Fri, 24 Jan 2025 16:21:08 +0530 Subject: [PATCH 5/5] Add lack forward TON amount without gas check example for TON smart contracts --- not-so-smart-contracts/ton/README.md | 9 +-- .../ton/fake_jetton_contract/README.md | 6 +- .../forward_ton_without_gas_check/README.md | 59 +++++++++++++++++++ 3 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 not-so-smart-contracts/ton/forward_ton_without_gas_check/README.md diff --git a/not-so-smart-contracts/ton/README.md b/not-so-smart-contracts/ton/README.md index 8a21e9ca..6249f9da 100644 --- a/not-so-smart-contracts/ton/README.md +++ b/not-so-smart-contracts/ton/README.md @@ -14,10 +14,11 @@ Each _Not So Smart Contract_ consists of a standard set of information: ## Vulnerabilities -| Not So Smart Contract | Description | -| -------------------------------------------- | ------------------------------------------------------- | -| [Int as Boolean](int_as_boolean) | Unexpected result of logical operations on the int type | -| [Fake Jetton contract](fake_jetton_contract) | Any contract can send a `transfer_notification` message | +| Not So Smart Contract | Description | +| -------------------------------------------------------------- | ----------------------------------------------------------- | +| [Int as boolean](int_as_boolean) | Unexpected result of logical operations on the int type | +| [Fake Jetton contract](fake_jetton_contract) | Any contract can send a `transfer_notification` message | +| [Forward TON without gas check](forward_ton_without_gas_check) | Users can drain TON balance of a contract lacking gas check | ## Credits diff --git a/not-so-smart-contracts/ton/fake_jetton_contract/README.md b/not-so-smart-contracts/ton/fake_jetton_contract/README.md index 3ef1798b..89b52b33 100644 --- a/not-so-smart-contracts/ton/fake_jetton_contract/README.md +++ b/not-so-smart-contracts/ton/fake_jetton_contract/README.md @@ -35,17 +35,17 @@ The following simplified code highlights the lack of token_id validation in the cell forward_payload_ref = in_msg_body~load_ref(); slice forward_payload = forward_payload_ref.begin_parse(); - int is_token0? = forward_payload.load_int(1); + int is_token0? = forward_payload~load_int(1); if (is_token0?) { slice balance_before = token0_balances.dict_get?(267, from_address); - int balance = balance_before.load_coins(); + int balance = balance_before~load_coins(); balance = balance + amount; slice balance_after = begin_cell().store_coinds(balance).end_cell().being_parse(); token0_balances~dict_set(267, from_address, balance_after); } else { slice balance_before = token1_balances.dict_get?(267, from_address); - int balance = balance_before.load_coins(); + int balance = balance_before~load_coins(); balance = balance + amount; slice balance_after = begin_cell().store_coinds(balance).end_cell().being_parse(); token1_balances~dict_set(267, from_address, balance_after); diff --git a/not-so-smart-contracts/ton/forward_ton_without_gas_check/README.md b/not-so-smart-contracts/ton/forward_ton_without_gas_check/README.md new file mode 100644 index 00000000..3168e3a4 --- /dev/null +++ b/not-so-smart-contracts/ton/forward_ton_without_gas_check/README.md @@ -0,0 +1,59 @@ +# Foward TON without gas check + +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. + +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. + +## Example + +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. + +```FunC +#include "imports/stdlib.fc"; + +() recv_internal(int my_balance, int msg_value, cell in_msg_full, slice in_msg_body) impure { + slice cs = in_msg_full.begin_parse(); + int flags = cs~load_uint(4); + ;; ignore all bounced messages + if (is_bounced?(flags)) { + return (); + } + slice sender_address = cs~load_msg_addr(); ;; incorrectly assumed to be Jetton wallet contract owned by this contract + + (int op, int query_id) = in_msg_body~load_op_and_query_id(); + + if (op == op::withdraw) { + int amount = in_msg_body~load_coins(); + slice to_address = in_msg_body~load_msg_addr(); + int forward_ton_amount = in_msg_body~load_coins(); ;; user specified forward TON amount + cell forward_payload = begin_cell().store_slice(in_msg_body).end_cell(); + + var msg_body = begin_cell() + .store_op_and_query_id(op::transfer, query_id) + .store_coins(amount) + .store_slice(to_address) + .store_slice(to_address) + .store_uint(0, 1) + .store_coins(forward_ton_amount) + .store_maybe_ref(forward_payload) + .end_cell(); + + cell msg = begin_cell() + .store_uint(0x18, 6) + .store_slice(USDT_WALLET_ADDRESS) + .store_coins(JETTON_TRANSFER_GAS + forward_ton_amount) ;; sending user specified forward TON amount + .store_uint(1, 1 + 4 + 4 + 64 + 32 + 1 + 1) ;; message parameters + .store_ref(ref) + .end_cell(); + + send_raw_message(msg, 0); + + return (); + } +} +``` + +## Mitigations + +- Avoid allowing users to specify forward TON amount with the outgoing messages. +- 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.