Unrestricted Access to Asset Data Modifcation in createLimitShort()

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/main/contracts/facets/ShortOrdersFacet.sol#L35-L48

Vulnerability details

Impact

In the contract ShortOrdersFacet.sol, the function createLimitShort does not validate the roles of the function caller and it allows any user to place short orders in the market. This could be problematic if the contract is intended to be used only by certain participants, like authorized traders or the contract owner.

A potential vulnerability associated with an unrestricted-access issue like this is that it may enable an attacker to manipulate the value of the associated asset or perform malicious actions to drain the liquidity pool. The immediate impact would be reduction of the validity of the prices of the assets in question, and possibly loss of funds, since the asset's value may be artificially lowered or raised by a malicious actor.

Proof of Concept

https://github.com/code-423n4/2024-03-dittoeth/blob/main/contracts/facets/ShortOrdersFacet.sol#L35-L48

function createLimitShort(
        address asset,
        uint80 price,
        uint88 ercAmount,
        MTypes.OrderHint[] memory orderHintArray,
        uint16[] memory shortHintArray,
        uint16 shortOrderCR
    ) external isNotFrozen(asset) onlyValidAsset(asset) nonReentrant {

    // ...

    // Anyone can create "empty" SR.
    incomingShort.shortRecordId = LibShortRecord.createShortRecord(asset, msg.sender, SR.Closed, 0, 0, 0, 0, 0);

    // ...

}

Tools Used

Manual review

Recommended Mitigation Steps

It's highly recommended to set role-based access control to restrict the function createLimitShort for security reasons. Utilize existing standards such as OpenZeppelin's Access Control library that offers a secure way to regulate which addresses can use specific operations in the contract.

Assessed type

Access Control

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

This isn't the intended design. It's being decentralized and open to everyone.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Insufficient proof