TempleGold
TempleDAO
Foundry
25,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

non whitelisted addresses can still recceive templeGold token

adeolu

Summary

function _update(address from, address to, uint256 value) internal override {
/// can only transfer to or from whitelisted addreess
/// @dev skip check on mint and burn. function `send` checks from == to

_update() is meant to only allow transfers to or from whitelisted addresses but it doesnt work as intended. a whitelisted address can transfer to an unwhitelisted address and an unwhitelisted address can make transfers to a whitelisted address

Vulnerability Details

https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/TempleGold.sol#L217C1-L224C6

function _update(address from, address to, uint256 value) internal override {
/// can only transfer to or from whitelisted addreess
/// @dev skip check on mint and burn. function `send` checks from == to
if (from != address(0) && to != address(0)) {
if (!authorized[from] && !authorized[to]) { revert ITempleGold.NonTransferrable(from, to); } //@audit uses "AND" logic, a whitelisted address can transfer to a non whitelisted address,
// a non whitelisted can transfer to non-whitelisted. should use "OR" logic to prevent any interaction from non whitelisted addresses
}
super._update(from, to, value);
}

This issue is caused by the use of AND operator instead of the OR operator in the logic. Because AND operator is used, both values in the condition have to be false for it to revert. So

  • when the from address is whitelisted and the to address is not whitelisted, the transfer will be sucessfull

  • when the to address is whitelisted and the from address isn't, the transfer will be sucessfull

This goes against the intended implementation which is to prevent transfers to and from unwhitelisted addresses.

Proof Of Concept

  • add code below to protocol/test/forge/templegold/TempleGold.t.sol

  • run test with forge test --mt test_transferToUnauthorized

function test_transferToUnauthorized() public {
address banned = makeAddr("banned");
​
vm.startPrank(executor);
templeGold.authorizeContract(banned, false);
vm.stopPrank();
​
vm.warp(block.timestamp + 2 days);
templeGold.mint();
​
​
vm.startPrank(teamGnosis); //team gnosis is already whitelisted
uint256 tokenAmount = templeGold.balanceOf(teamGnosis);
//vm.expectRevert(abi.encodeWithSelector(ITempleGold.NonTransferrable.selector, teamGnosis, alice));
templeGold.transfer(banned, tokenAmount); //so now whitelisted address transfers to banned account, the whitelist check is not sufficient
assertEq(tokenAmount, templeGold.balanceOf(banned)); //assert that banned address received tokens
vm.stopPrank();
​
​
//now transfer from unwhitelisted/banned to whitelisted address
vm.prank(banned);
templeGold.transfer(teamGnosis, tokenAmount); //ideally this should revert because it is banned address making transfer and should not be able to transfer but it doesnt
assertEq(tokenAmount, templeGold.balanceOf(teamGnosis));
assertEq(0, templeGold.balanceOf(banned)); //asserts that banned account made a transfer
​
}

Impact

Whitelist is not enforced in all cases. Still possible to transfer to an unwhitelisted address. Still possible to transfer from unwhitelsited address to whitelisted address.

Tools Used

manual review, foundry

Recommendations

change the AND operartor to OR, this will allow reverts if to or from is not whitelisted.

function _update(address from, address to, uint256 value) internal override {
/// can only transfer to or from whitelisted addreess
/// @dev skip check on mint and burn. function `send` checks from == to
if (from != address(0) || to != address(0)) {
if (!authorized[from] && !authorized[to]) { revert ITempleGold.NonTransferrable(from, to); }
}
super._update(from, to, value);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Appeal created

adeolu Submitter
11 months ago
inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.