0xBhumii - use of tx.origin is not recommonded

[sherlock-admin]

0xBhumii

high

use of tx.origin is not recommonded

Summary

In GSPTrader contract , sellBase , sellQuote and flashLoan functions are using tx.origin which is not a good practice and exposes the contract to attacks

Vulnerability Detail

The use oftx.origin poses a potential vulnerability by referencing the initial external account or contract that initiated the transaction. While tx.origin identifies the first external entity that triggered the transaction, it may not represent the ultimate initiator in complex transaction chains. For instance, consider a scenario where Contract A initiates an external call to Contract B, and then Contract B, in turn, initiates another external call to Contract C. In this multistep transaction chain, Contract C, if using tx.origin for verification purposes, would retrieve the address of Contract A as the transaction initiator, rather than Contract B, which directly interacts with Contract C. This discrepancy in identification can lead to incorrect assumptions about the transaction's origin, potentially causing: Misattributed Permissions: Contracts relying on tx.origin might grant or deny permissions based on an incorrect assumption of the transaction initiator. This can lead to unauthorized access or incorrect authorization decisions.

Security Risks: Verification or access control mechanisms based on tx.origin might mistakenly assume certain security contexts, opening the contract to exploitation or unexpected behavior from external contracts.

In the GSPTrader contract also, tx.origin is used in multiple functions like sellBase , sellQuote and flashLoan which exposes the contract to a lot of risks.

Impact

Incorrect Attribution: tx.origin may misattribute the initiator of a transaction, potentially leading to incorrect assumptions about the true originator, especially in complex contract interactions. Security Risks: Relying on tx.origin for access control or authentication can introduce vulnerabilities, enabling unauthorized access or granting incorrect permissions. Exploitation: Malicious actors might manipulate transaction chains to deceive contracts relying on tx.origin, leading to unexpected behaviors or unauthorized actions.

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPTrader.sol#L47 https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPTrader.sol#L87 https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPTrader.sol#L152

Tool used

Manual Review

Recommendation

use msg.sender instead. because msg.sender represent the last account/contract that initiated the transaction unlike tx.origin reducing various security risks .

[nevillehuang]

Invalid, tx.origin is determined by the user calling, so no issue here as it is their responsibility to call from a valid address when buying/seller shares