From 7b18e132b946583b000ff50f0d9df817e307de7a Mon Sep 17 00:00:00 2001 From: Tarun Bansal Date: Fri, 24 Jan 2025 16:21:08 +0530 Subject: [PATCH] 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.