* Project name: Request Escrow
* Auditor name: Joshua Evuetapha
* The commit hash used for auditing: https://github.com/RequestNetwork/requestNetwork/blob/53ebc7a54a610ac28f616f87e1bda6f3d1e3265f/packages/smart-contracts/src/contracts/ERC20EscrowToPay.sol
* List of spec/docs: https://support.request.finance/essentials/escrow
* List of issues
1. * Severity: High
* Title: Potential Malicious Code with External Contract.
* Description: In Solidity any address can be casted into specific contract, even if the contract at the address is not the one being casted.
* File(s): ERC20EscrowToPay.sol (Line 142 -144).
* Exploit (optional): - This contract may be initialized with a malicious code at the constructor.
* Recommendation: The address of external contract should be verified and audited.
2. * Severity: High
* Title: Reentrancy
* Description: A malicious contract can reenter the `payRequestFromEscrow` function and make multiple withdrawals.
* File(s): ERC20EscrowToPay.sol (Line 240 - 248).
* Exploit (optional): -
* Recommendation: Use function modifiers that prevent re-entrancy.
3. * Severity: High
* Title: FeeAddress and FeeAmount can be manipulated.
* Description: The `payEscrow` function accept `feeAddress` and `feeAmount` as arguments, this can lead to users setting any address receive fees, they can also set zero `feeAmount` thereby paying no fees.
* File(s): ERC20EscrowToPay.sol (Line 175 -208).
* Exploit (optional): -
* Recommendation: Fees address should be a state variable in the smart contract that only owner can set, and the fees amount should be a percentage of the `_amount`.
4. * Severity: Medium
* Title: No validation for `_tokenAddress` and `to` Addresses
* Description: There is a possibility of passing `address(0)` as the `_tokenAddress` or `_to` address as parameters to the `payRequestFromEscrow` function .
* File(s): ERC20EscrowToPay.sol (Line 175 - 208).
* Exploit (optional): -
* Recommendation: The `_tokenAddress` and `to` should be checked to make sure they are not `address(0)`.
5. * Severity: Low
* Title: Transfering zero tokens to `address(0)`
* Description: Line 362 and 363 attempts to send 0 tokens to `address(0)`, this is a redundant operation that will cost more gas than needed.
* File(s): ERC20EscrowToPay.sol (Line 348)
* Exploit (optional): -
* Recommendation: Create a new function that transfers tokens to the payee only.
6. * Severity: Informational
* Title: Unlocked Pragma
* Description: Every Solidity file specifies in the header a version number of the format `pragma solidity (^)0.8.4`. The caret (`^`) before the version number implies an unlocked pragma, meaning that the compiler will use the specified version and above, hence the term "unlocked".
* File(s): ERC20EscrowToPay.sol
* Exploit (optional): -
* Recommendation: For consistency and to prevent unexpected behavior in the future, we recommend to remove the caret to lock the file onto a specific Solidity version.
7. * Severity: Informational
* Title: Ignored return value from interaction with an external contract
* Description: The return value of `erc20.safeApprove(address(paymentProxy), max)` was ignored this could lead to unexpected behaviour.
* File(s): ERC20EscrowToPay.sol (Line 348)
* Exploit (optional): -
* Recommendation: Get the return value from that interaction, if it's true proceed, else revert with an error
* Documentation issues (section)
* The `feeAddress` and `feeAmount` were not adequately discussed in the documentation, how is the `feeAmount` generated and who own's the `feeAddress`.
* The functions `setEmergencyClaimPeriod` and `setFrozenPeriod` were not documented.