tx.origin may be removed in future and its usage is not recommended

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateUP/LSP1UniversalReceiverDelegateUP.sol#L103

Vulnerability details

Impact

There is a chance that tx.origin will be removed from the Ethereum protocol in the future, so code that uses tx.origin must be avoid for the authentication purpose.

There is also some EIPs being proposed for change/remove of tx.origin.

https://github.com/ethereum/EIPs/issues/637

https://www.reddit.com/r/ethereum/comments/6d11lv/erc_about_txorigin_change_for_account_abstraction/

In LSP1UniversalReceiverDelegateUP.sol,

File: contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateUP/LSP1UniversalReceiverDelegateUP.sol

78    function universalReceiver(
79        bytes32 typeId,
80        bytes memory /* data */
81    ) public payable virtual returns (bytes memory result) {

     // Some code

103     if (notifier == tx.origin) revert CannotRegisterEOAsAsAssets(notifier);

     // Some code

143 }

Here, the function has used tx.origin at L-103 for authentication but these can not work in future if the new EIPs concerning to remove tx.origin removal due to security issues got approved and tx.origin is removed from Ethereum protocol.

Proof of Concept

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateUP/LSP1UniversalReceiverDelegateUP.sol#L103

Tools Used

Manual review

Recommended Mitigation Steps

Use msg.sender instead of tx.origin.

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-pre-sort]

minhquanym marked the issue as low quality report

[minhquanym]

OOS in the winning bot race submission

[c4-judge]

trust1995 marked the issue as unsatisfactory: Out of scope