Audinarey - bridged funds will be stuck without a way to withdraw.

[sherlock-admin2]

Audinarey

high

bridged funds will be stuck without a way to withdraw.

Summary

Users can call bridgePool(...) to deposit funds to an L2 contract. Although the _l2TxGasLimit is specified, but the _l2TxGasPerPubdataByte is hardcoded to 0 and as such the bridged funds will get stuck without a way to withdraw. This means that gas is not sent with the transaction to the L2 for the execution of the deposit on the L2 and as such the deposit may get stuck in the L2 bridge contract.

File: SophonFarming.sol
748:     function bridgePool(uint256 _pid) external {
...
764:         // TODO: change _refundRecipient, verify l2Farm, _l2TxGasLimit and _l2TxGasPerPubdataByte
765:         // These are pending the launch of Sophon testnet
766:         bridge.deposit(
767:             pool.l2Farm,            // _l2Receiver
768:             address(lpToken),       // _l1Token
769:             depositAmount,          // _amount
770             200000,                 // _l2TxGasLimit
771:   @>        0,                      // _l2TxGasPerPubdataByte
772:             owner()                 // _refundRecipient
773:         );

Vulnerability Detail

Impact

Funds sent via bridge will get stuck without actually depositing in the L2 contract

Code Snippet

https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L748-L775

Tool used

Manual Review

Recommendation

Dont hard code the_l2TxGasPerPubdataBytevalue in thebridgePool(...)` function

[sherlock-admin3]

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

invalid because bridge logic not implemented and not covered by this audit

[Audinarey]

Escalate

invalid because bridge logic not implemented and not covered by this audit

The codebase is the source of truth in this case, the functionality in question is in scope of this audit and the comments gives us pointers as to the intended purpose of the fuction and its parameters.

III. Sherlock's standards: Hierarchy of truth: If the protocol team provides no specific information, the default rules apply (judging guidelines).

If the protocol team provides specific information in the README or CODE COMMENTS, that information stands above all judging rules.

1)

    /**
@>   * @notice Permissionless function to allow anyone to bridge during the correct period
     * @param _pid pid to bridge
     */
    function bridgePool(uint256 _pid) external {

        bridge.deposit(
            pool.l2Farm,            // _l2Receiver
            address(lpToken),       // _l1Token
            depositAmount,          // _amount
            200000,                 // _l2TxGasLimit
    @>      0,                      // _l2TxGasPerPubdataByte
            owner()                 // _refundRecipient
        );

[sherlock-admin3]

Escalate

invalid because bridge logic not implemented and not covered by this audit

The codebase is the source of truth in this case, the functionality in question is in scope of this audit and the comments gives us pointers as to the intended purpose of the fuction and its parameters.

III. Sherlock's standards: Hierarchy of truth: If the protocol team provides no specific information, the default rules apply (judging guidelines).

If the protocol team provides specific information in the README or CODE COMMENTS, that information stands above all judging rules.

1)

    /**
@>   * @notice Permissionless function to allow anyone to bridge during the correct period
     * @param _pid pid to bridge
     */
    function bridgePool(uint256 _pid) external {

        bridge.deposit(
            pool.l2Farm,            // _l2Receiver
            address(lpToken),       // _l1Token
            depositAmount,          // _amount
            200000,                 // _l2TxGasLimit
    @>      0,                      // _l2TxGasPerPubdataByte
            owner()                 // _refundRecipient
        );

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[mystery0x]

Please see sponsor's comment on https://github.com/sherlock-audit/2024-05-sophon-judging/issues/147 that's similarly reported to your report:

"bridge is not implemented yet. this is just a marketing stub. we will have separate audit for the bridge when we develop it"

[Audinarey]

Sponsor's comment does not hold water in this case considering sherlock rules

If the protocol team provides specific information in the README or CODE COMMENTS, that information stands above all judging rules.

Market stub or not, The rule applies here

[brakeless-wtp]

"Any bridging related code is considered out of scope"

It is clearly stated in the README Q&A that bridging code is OOS.

[Audinarey]

@mystery0x I await your response

[WangSecurity]

As correctly mentioned above, the README clearly says:

Any bridging related code is considered out of scope

In the additional audit info question.

Planning to reject the escalation and leave the issue as it is, since it's out of scope.

[WangSecurity]

Result: Invalid Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• Audinarey: rejected