No Allowance or Permit set before safeTransferFrom
Summary
SafeTransferFrom is used in BuyOrderIssuer._requestOrderAccounting(), DirectBuyIssuer.returnEscrow(), SellOrderProcessor._requestOrderAccounting() and SellOrderProcessor._fillOrderAccounting() but no Allowance/Approve/Permit is set before the function is called.
Vulnerability Detail
In the mentioned functions, safeTransferFrom is used to transfer tokens from msg.sender to the contract. However, before making a transfer from a user's account, the contract should ensure that it has been given an allowance by the user through allowance, approve or permit. This is a standard pattern in ERC20 tokens that protects user's tokens. This step is missing which means that these functions will fail or could be an oversight that could lead to other security issues.
Impact
This would prevent the core functionalities of the contract from being executed as _requestOrderAccounting(), _fillOrderAccounting() and returnEscrow() would fail. This could also be an oversight that could lead to other security issues.
Before calling safeTransferFrom function to move tokens from a user's address, ensure that an appropriate allowance has been set by the token holder. This can be achieved by instructing users to call the safeIncreaseAllowance method. Additionally, the contract could implement and expose a permit function which can set the allowance in a single transaction along with the transfer, for a better user experience. Implement checks to ensure that the contract has the necessary allowance to perform token transfers on behalf of the user. This would prevent transaction failures and protect against potential security vulnerabilities.
nisedo
high
No Allowance or Permit set before safeTransferFrom
Summary
SafeTransferFrom is used in BuyOrderIssuer._requestOrderAccounting(), DirectBuyIssuer.returnEscrow(), SellOrderProcessor._requestOrderAccounting() and SellOrderProcessor._fillOrderAccounting() but no Allowance/Approve/Permit is set before the function is called.
Vulnerability Detail
In the mentioned functions, safeTransferFrom is used to transfer tokens from msg.sender to the contract. However, before making a transfer from a user's account, the contract should ensure that it has been given an allowance by the user through allowance, approve or permit. This is a standard pattern in ERC20 tokens that protects user's tokens. This step is missing which means that these functions will fail or could be an oversight that could lead to other security issues.
Impact
This would prevent the core functionalities of the contract from being executed as _requestOrderAccounting(), _fillOrderAccounting() and returnEscrow() would fail. This could also be an oversight that could lead to other security issues.
Code Snippet
https://github.com/sherlock-audit/2023-06-dinari/blob/main/sbt-contracts/src/issuer/BuyOrderIssuer.sol#L140-L141
https://github.com/sherlock-audit/2023-06-dinari/blob/main/sbt-contracts/src/issuer/DirectBuyIssuer.sol#L96-L97
https://github.com/sherlock-audit/2023-06-dinari/blob/main/sbt-contracts/src/issuer/SellOrderProcessor.sol#L87-L88
https://github.com/sherlock-audit/2023-06-dinari/blob/main/sbt-contracts/src/issuer/SellOrderProcessor.sol#L116-L117
Tool used
Manual Review
Recommendation
Before calling safeTransferFrom function to move tokens from a user's address, ensure that an appropriate allowance has been set by the token holder. This can be achieved by instructing users to call the safeIncreaseAllowance method. Additionally, the contract could implement and expose a permit function which can set the allowance in a single transaction along with the transfer, for a better user experience. Implement checks to ensure that the contract has the necessary allowance to perform token transfers on behalf of the user. This would prevent transaction failures and protect against potential security vulnerabilities.