TVM-Solidity-Compiler
TVM-Solidity-Compiler copied to clipboard
Proposal: force users to explicitly specify responsible answer flags.
Hi there,
I will start with two problems with default responsible.
Problem #1, default responsible has flag: 0 and he send value depend on pragma msgValue? 7_500_000 or 9_500_000.
Simple contract:
contract SimpleContract {
address static owner;
constructor() public {
tvm.accept();
}
function get_owner() external pure responsible returns (address) {
return owner;
}
}
Get owner will be compiled to
function get_owner() external pure responsible returns (address) {
return {flag: 0, value: 7_500_00, bounce: true} owner;
}
Also it can be used to make money. If attacker find contract with such responsible he can pay for call responsible < 7_500_00 evers and get back 7_500_00 evers. So he can drain balance. This will getting worse if gas price will be lower in future.
Problem #2, default responsible has bounce: true, and it is really bad.
Example of vulnerable contract (pseudocode):
pragma ton-solidity >= 0.39.0;
pragma AbiHeader expire;
pragma AbiHeader pubkey;
contract VulnerableContract {
uint128 balance;
function get_balance() override external view responsible returns (uint32) {
return{flag: 64, value: 0} balance;
}
function transfer(uint128 _amount_tokens, uint256 _to_pubkey) external {
require(tvm.pubkey() == msg.pubkey());
require(balance > _amount_tokens);
tvm.accept();
address reciever = _getExpectedAddress(_to_pubkey);
balance -= _amount_tokens;
VulnerableContract(reciever).recieve{value: 7_500_000, flag: 0, bounce: true}(_amount_tokens, tvm.pubkey());
}
function recieve(uint128 _amount_tokens, uint256 _from_pubkey) {
address expected_sender = _getExpectedAddress(_from_pubkey)
require(msg.sender == expected_sender);
balance += _amount_tokens
}
onBounce(TvmSlice body) external {
tvm.accept();
uint32 functionId = body.decode(uint32);
if (functionId == tvm.functionId(VulnerableContract.transfer)) {
uint128 amount_tokens = body.decode(uint128);
balance += amount_tokens;
}
}
}
Attacker can call get_balance with answerID = tvm.functionId(VulnerableContract.transfer) and throw exception in callback. In this case VulnerableContract will receive unexpected onBounce and will double their tokens.
To solve this problems my suggestion is to force users to set ALL three params explicitly. Just throw compile time error if one of (value, flag, bounce) is not set.
Thank you! Yes, it's good hint!
Resolved in 0.60.0