Payment Processors treat calls to non-contracts as a successful transfer calls

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xd85afafb7ae476849876672d4f09e931e916b482e986843a8596618a232f076c Severity: medium

Description:

Description

PP_Simple_v1.sol and PP_Streaming_v1.sol contain raw calls to a token contract at lines 126 and 726, respectively. These calls might be treated as a successful transfer when the token_ address is not a contract. This happens because the call to a non-contract address returns true.

As a result, the following line:

if (success && (data.length == 0 || abi.decode(data, (bool))))

will pass. Consequently, the TokensReleased event will be emitted, and the client will be notified about the paid tokens.

Problem 1

This issue opens the possibility of phishing attacks due to the incorrect flow of the code being executed and the TokensReleased event being emitted. This might include token addresses that are yet-to-be-deployed, i.e. with the use of create2 or LP tokens for the undeployed pools of new tokens. The contract will notify the client that the tokens were paid, while that was not the case.

Problem 2

Due to the modular nature of the project, this code implies hidden checks in other parts (i.e., the token is a contract) since the flow of a successful transfer is executed instead of a failed call. This might cause problems when new modules are integrated with the current one.

Suggestion to fix\ I recommend using SafeERC20 library from the OpenZeppelin.

Attachments

1. Proof of Concept (PoC) File inverter-issue.sol

Files:

• inverter-issue.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmT8MhMVtaxR2oD36VWt5LBf5Ahd2CUmdkp4EQssFH9Toh)

[PlamenTSV]

The tokens that are streamed are chosen by the Orchestrator, e.g orchestrator().fundingManager().token()

[FHieser]

No with the addition of the PaymentToken in the PaymentOrder we actually allow for different Tokens to be paid out instead of the fundingmanager token. We are planning to do a check on paymentOrder Creation anyway. we will add a check for code size there probably