TempleGold

Summary

The function TempleGold::_update does not check properly the addresses of the token transaction, thus allowing authorized addresses to transfer to non authorized addresses and viceversa

Vulnerability Details

The function _update allows to transfer tokens.

However, the marked line does not properly filter the update transactions. I am going to prove it mathematically:

&& is the AND logic operator of Solidity. It only returns true if BOTH values checked are true, and returns false if not.

The "authorized" mapping contains "true" if the address is authorized and "false" if it is not.

If this conditional returns "true", the transaction reverts and continues otherwise. Therefore, it must return false if both addresses are authorized and true if either of them is not authorized.

We have 4 cases.
Using this notation : (from,to) where "from" are "to" are boolean values:

Case 1:
Both are authorized : (true, true)
!(true) && !(true) = false && false = false
the condition works fine here, transaction continues

Case 2:
Both are not authorized : (false,false)
!(false) && !(false) = true && true = true
the condition works fine here, transaction reverts

Case 3:
Only the sender address is authorized : (true, false)
!(true) && !(false) = false && true = false
the condition does NOT work fine here, transaction continues when it should revert (destination is NOT authorized)

Case 4:
Only the destination is authorized : (false,true)
!(false) && !(true) = true && false = false
the condition does NOT work fine here, transaction continues when it should revert (sender is NOT authorized)

We have proven that this logic operator does NOT filter properly the transfers.

Impact

Unauthorized address are able to obtain the token.

Tools Used

Manual Review

Recommendations

Changing the logic operator AND for the logic operator OR would solve this issue
The OR logic operator is represented in || in solidity, and returns true if at least one of the values is true.

I will prove that this operator solves the problem using the same starting conditions as before:

We have 4 cases.
Using this notation : (from,to) where "from" are "to" are boolean values:

Case 1:
Both are authorized : (true, true)
!(true) || !(true) = false || false = false
the condition works fine here, transaction continues

Case 2:
Both are not authorized : (false,false)
!(false) || !(false) = true || true = true
the condition works fine here, transaction reverts

Case 3:
Only the sender address is authorized : (true, false)
!(true) || !(false) = false || true = true
the condition works fine here, transaction reverts

Case 4:
Only the destination is authorized : (false,true)
!(false) || !(true) = true || false = true
the condition works fine here, transaction reverts

The change in the code is simple: