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

x/gamm: Add check to ensure JoinPoolNoSwap cannot be called with arbitrary denoms #1929

Closed
AlpinYukseloglu opened this issue Jul 1, 2022 · 0 comments · Fixed by #1930
Closed
Assignees
Labels
C:x/gamm Changes, features and bugs related to the gamm module. T:bug 🐛 Something isn't working

Comments

@AlpinYukseloglu
Copy link
Contributor

Background

Our current implementation of JoinPoolNoSwap only checks for whether the input set of tokens includes the tokens in the pool, but does not have any check that prevents someone from putting in an arbitrary number of arbitrary tokens. Since this is a no-swap/all-asset join, there are no other immediate checks so an invalid input can make it directly to the state-changing JoinPool function.

func (k Keeper) JoinPoolNoSwap(

An example of how this could potentially be exploited for the OSMO/USDC pool (1 OSMO = 1 USDC):

  1. Create arbitrary spam/poisoned token FOO
  2. Create a JoinPoolNoSwap tx and send in a set of [1 OSMO, 1 USDC, 1 FOO] as your input
  3. The following check doesn't catch the existence of FOO, since we only check if the input includes 1 OSMO and 1 USDC:
    if !(neededLpLiquidity.DenomsSubsetOf(tokenInMaxs) && tokenInMaxs.IsAllGTE(neededLpLiquidity)) {

Suggested Design

  • Add && tokenInMaxs.DenomSubsetOf(neededLpLiquidity) to x/gamm/keeper/pool_service.go/L203 so that input tokens for a no-swap pool join don’t include denoms that aren’t in the pool:
    if !(neededLpLiquidity.DenomsSubsetOf(tokenInMaxs) && tokenInMaxs.IsAllGTE(neededLpLiquidity)) {

Acceptance Criteria

  • All existing and new tests pass
@AlpinYukseloglu AlpinYukseloglu added the C:x/gamm Changes, features and bugs related to the gamm module. label Jul 1, 2022
@AlpinYukseloglu AlpinYukseloglu self-assigned this Jul 1, 2022
@ValarDragon ValarDragon moved this to Needs Review 🔍 in Osmosis Chain Development Jul 1, 2022
@AlpinYukseloglu AlpinYukseloglu added the T:bug 🐛 Something isn't working label Jul 1, 2022
Repository owner moved this from Needs Review 🔍 to Done ✅ in Osmosis Chain Development Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/gamm Changes, features and bugs related to the gamm module. T:bug 🐛 Something isn't working
Projects
Archived in project
1 participant