Metamask Snap Audit Report for Xtreamly

Management Summary

Xtreamly contacted Sayfer Security in order to perform penetration testing on their MetaMask Snap in April 2024.

Before assessing the above services, we held a kickoff meeting with the Xtreamly technical team and received an overview of the system and the goals for this research.

Over the research period of 2 weeks, we discovered 6 vulnerabilities in the system.

In conclusion, several fixes should be implemented following the report, but the system’s security posture is competent.

After a review by the Sayfer team, we certify that all the security issues mentioned in this report have been addressed by the Xtreamly team.

Risk Methodology

At Sayfer, we are committed to delivering the highest quality penetration testing to our clients. That’s why we have implemented a comprehensive risk assessment model to evaluate the severity of our findings and provide our clients with the best possible recommendations for mitigation.

Our risk assessment model is based on two key factors: IMPACT and LIKELIHOOD. Impact refers to the potential harm that could result from an issue, such as financial loss, reputational damage, or a non-operational system. Likelihood refers to the probability that an issue will occur, taking into account factors such as the complexity of the attack and the number of potential attackers.

By combining these two factors, we can create a comprehensive understanding of the risk posed by a particular issue and provide our clients with a clear and actionable assessment of the severity of the issue. This approach allows us to prioritize our recommendations and ensure that our clients receive the best possible advice on how to protect their business.

Risk is defined as follows:

Security Evaluation Methodology

Sayfer uses OWASP WSTG as our technical standard when reviewing web applications. After gaining a thorough understanding of the system we decided which OWASP tests are required to evaluate the system.

Security Assessment

After understanding and defining the scope, performing threat modeling, and evaluating the correct tests required in order to fully check the application for security flaws, we performed our security assessment.

Issue Table Description

Issue title

Description

Here we provide a brief description of the issue and how it formed, the steps we made to find or exploit it, along with proof of concept (if present), and how this issue can affect the product or its users.

Mitigation

Suggested resolving options for this issue and links to advised sites for further remediation.

Security Assessment Findings

Simulation of Multihop Swaps May Be Inaccurate

Description

index.ts uses firstPath.tokenIn and lastPath.tokenOut, the very first and the very last tokens, to construct the GET query parameters in getQuote() and predictSlippage().

• index.ts:33-34

const firstPath: SwapPath = decoded.path[0]
const lastPath: SwapPath = decoded.path[decoded!.path.length - 1]

• index.ts:55-74

const [{ quotedPrice, poolAddress, _ }, predictedSlippage] = await
Promise.all([
    getQuote(
        tokenIn?.address,
        tokenOut?.address,
        amountIn,
        tokenIn?.decimals,
        tokenOut?.decimals,
        firstPath?.fee),
    predictSlippage(
        tokenIn?.address,
        tokenOut?.address,
        amountIn,
        tokenIn?.decimals,
        tokenOut?.decimals,
        // Whether its a buy or sell
        tokenIn?.address != WETH_TOKEN_ETH.address,
        firstPath.fee,
        getTradingSymbol(tokenIn!, tokenOut!),
    )
])

These two functions then use them to construct their parameters.

• server.ts:6; predictSlippage(string, string, number, number, number, boolean, number, string)

const url =
`${PredictSlippageAPIUrl}?tokenInAddress=${tokenInAddress}&tokenOutAddress=${tokenOutAddress}&amountIn=${amountIn}&decimalIn=${decimalIn}&decimalOut=${decimalOut}&fee=${fee}&isBuy=${isBuy}&symbol=${symbol}`

• server.ts:27; getQuote(string, string, number, number, number, number)

const url =
`${QuotePriceAPIUrl}?tokenInAddress=${tokenInAddress}&tokenOutAddress=${tokenOutAddress}&amountIn=${amountIn}&decimalIn=${decimalIn}&decimalOut=${decimalOut}&fee=${fee}`

However, uniswap allows multihop swaps, where users specify a certain swap path to use (see here). A calculation of potential slippage when a custom swap is used may be inaccurate without considering the full path.

Mitigation

Either do not support multihop swaps if there are multiple paths provided in the transaction data, or allow the querier/slippage calculation API to consider all specified paths, instead of just first and last tokens.

Dependencies Are not Pinned to Exact Versions

Description

Some of the external dependencies used by the snap are not pinned to exact version numbers, but are set to floating compatible versions (i.e. ^x.x.x). This can potentially enable dependency attacks.

Mitigation

Pinning dependencies to an exact version (i.e. =x.x.x) can reduce the possibility of inadvertently introducing a malicious version of a dependency in the future.

Potentially vulnerable dependencies

Description

According to npm audit, package ses which is used by the snap contains a critical vulnerability that allows for arbitrary code execution.

While there is no guarantee that just relying on such a package is a sufficient condition to an exploit, it is recommended to update any vulnerable dependencies.

Mitigation

Update the affected package. Follow the detailed description provided in the advisory linked above.

Lack of Unit Test Coverage

Description

The snap does not have any kind of testing scenarios covered. There are no unit tests included in the repo. Unit tests are important to explore common usage scenarios and make sure they work before deploying.

Mitigation

We recommend creating and maintaining testing scenarios covering as much code as possible.

Leftover Testing Code

Description

index.ts has some console.log calls which seem to be a leftover testing code rather than desired product feature.

• index.ts:48-49

console.log(firstPath)
console.log(lastPath)

Mitigation

This code can be safely removed in production builds.

Incorrect Dependency Version

Description

The dependency buffer has version ^.0.3 instead of ^6.0.3. The npm site for buffer clearly points that the correct version is 6.0.3.

Mitigation

Change the required version to 6.0.3.