Lines of code

https://github.com/code-423n4/2024-11-nibiru/blob/84054a4f00fdfefaa8e5849c53eb66851a762319/x/evm/precompile/oracle.go#L87

Vulnerability details

Finding description and impact

The code in the Oracle precompile's exchange rate query functionality to properly handle asset pair strings containing colons. The issue occurs because the system uses colons (:) as delimiters for asset pairs, but colons are also valid characters in Cosmos SDK denominations.

This issue manifests in the queryExchangeRate function where TryNewPair() is called to parse the asset pair string. The function splits the input string on the : character, assuming it's only used as a delimiter between the base and quote assets. However, Cosmos SDK's ValidateDenom() allows colons in denomination names because of this validation regex in coin.go (within the Cosmos SDK):

	// Denominations can be 3 ~ 128 characters long and support letters, followed by either
	// a letter, a number or a separator ('/', ':', '.', '_' or '-').
	reDnmString = `[a-zA-Z][a-zA-Z0-9/:._-]{2,127}`

This means that whenever the precompile is used to query such a (valid) Cosmos SDK denom, it will return an error and it will never be possible to use these denoms when a price is needed.

Proof of Concept

The issue exists in the following code path:

1. In oracle.go:87:

assetPair, err := asset.TryNewPair(pair)

2. This calls into pair.go:25-41:

func TryNewPair(pair string) (Pair, error) {
    split := strings.Split(pair, ":")
    splitLen := len(split)
    if splitLen != 2 {
        if splitLen == 1 {
            return "", sdkerrors.Wrapf(ErrInvalidTokenPair,
                "pair separator missing for pair name, %v", pair)
        } else {
            return "", sdkerrors.Wrapf(ErrInvalidTokenPair,
                "pair name %v must have exactly two assets, not %v", pair, splitLen)
        }
    }
    // ...
}

The validation within TryNewPair returns an error when the split returns more than two entries.

Consider the following example that demonstrates the issue:

// Valid Cosmos SDK denomination containing a colon
validDenom := "ibc:transfer/channel-0/unibi"
quoteDenom := "usd"

// Attempt to create pair "ibc:transfer/channel-0/unibi:usd"
pair := validDenom + ":" + quoteDenom

// This will fail because strings.Split() will create three parts:
// ["ibc", "transfer/channel-0/unibi", "usd"]
// TryNewPair will return an error since it expects exactly 2 parts
assetPair, err := asset.TryNewPair(pair)
// err will be "pair name must have exactly two assets, not 3"

This demonstrates how a valid denomination name containing a colon will be incorrectly split and rejected, even though it should be a valid asset pair according to Cosmos SDK denomination rules.

Recommended mitigation steps

There are several possible approaches to mitigate this issue:

1. Use a different delimiter:
   Replace the colon delimiter with a character that's guaranteed to be invalid in Cosmos SDK denominations.

2. Implement escape sequence handling:
   Modify the parsing logic to handle escaped colons in denomination names:

func TryNewPair(pair string) (Pair, error) {
    // Replace escaped colons with a temporary placeholder
    escaped := strings.ReplaceAll(pair, "\\:", "\x00")
    
    // Split on unescaped colons
    parts := strings.Split(escaped, ":")
    
    // Restore escaped colons
    for i := range parts {
        parts[i] = strings.ReplaceAll(parts[i], "\x00", ":")
    }
    
    // Continue with validation...
}

3. Use structured input:
   Instead of using string concatenation with delimiters, modify the interface to accept base and quote denominations as separate parameters:

func NewPair(base string, quote string) Pair {
    return Pair{
        BaseDenom:  base,
        QuoteDenom: quote,
    }
}