Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

anyone can change the global data uniswapfactory contracts #309

Closed
c4-bot-1 opened this issue Jun 10, 2024 · 2 comments
Closed

anyone can change the global data uniswapfactory contracts #309

c4-bot-1 opened this issue Jun 10, 2024 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden insufficient quality report This report is not of sufficient quality 🤖_56_group AI based duplicate group recommendation

Comments

@c4-bot-1
Copy link
Contributor

c4-bot-1 commented Jun 10, 2024

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/libraries/logic/AddPairLogic.sol#L44

Vulnerability details

Proof of Concept

The initializeGlobalData function is marked as external, meaning it can be called from outside the contract. This design choice allows external contracts or accounts to invoke this function. The purpose of initializeGlobalData is to initialize global data, which includes setting the uniswapFactory address. Global data often contains critical information that governs the behavior and interactions of the entire system.
Despite the importance of the uniswapFactory address and the sensitivity of global data, there are no access control mechanisms implemented within the initializeGlobalData function. This means that any external entity with knowledge of the AddPairLogic contract address and access to the GlobalDataLibrary.GlobalData struct can potentially invoke this function and modify the uniswapFactory address.

    function initializeGlobalData(GlobalDataLibrary.GlobalData storage global, address uniswapFactory) external {
        global.pairsCount = 1;
        global.vaultCount = 1;
        global.uniswapFactory = uniswapFactory;
    }

An attacker could exploit this vulnerability by deploying a malicious contract or by gaining control of an existing contract within the system. Once control is established, the attacker could call initializeGlobalData with a malicious uniswapFactory address, thereby compromising the integrity and functionality of the system. For example, the attacker could substitute a legitimate Uniswap factory address with a malicious one, leading to unintended interactions with malicious contracts or loss of funds.

Impact

Since the uniswapFactory address is utilized in functions like addPair for verifying the validity of Uniswap pools, unauthorized modification of this address could result in the addition of invalid or malicious token pairs. This could lead to unforeseen interactions with counterfeit Uniswap pools, potentially causing disruptions in trading activities, loss of funds, or even manipulation of asset prices within the system. Consequently, the integrity and reliability of the entire token pair addition process would be compromised, posing significant risks to users and the stability of the platform.

Tools Used

Manual

Recommended Mitigation Steps

Implement access control mechanisms to restrict the invocation of the initializeGlobalData function to only trusted contracts,addresses or make initializeGlobalData can be called by only once when initialize the contracts .

Assessed type

Access Control

@c4-bot-1 c4-bot-1 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jun 10, 2024
c4-bot-5 added a commit that referenced this issue Jun 10, 2024
@c4-bot-12 c4-bot-12 added the 🤖_56_group AI based duplicate group recommendation label Jun 14, 2024
@howlbot-integration howlbot-integration bot added the insufficient quality report This report is not of sufficient quality label Jun 17, 2024
@Odhiambo526
Copy link

@alex-ppg any reason for this getting invalidated?

@alex-ppg
Copy link

alex-ppg commented Jul 4, 2024

Hey @Odhiambo526, thanks for contributing to the PJQA process! This represents a validation repository finding and as such was not evaluated by me directly.

The AddPairLogic implementation is a library, not a contract, thereby disallowing direct calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden insufficient quality report This report is not of sufficient quality 🤖_56_group AI based duplicate group recommendation
Projects
None yet
Development

No branches or pull requests

5 participants