code-423n4 / 2023-06-canto-findings

1 stars 0 forks source link

GetAllPools could panic at iterator loop #104

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/coinswap/keeper/pool.go#L50

Vulnerability details

Impact

A panic could occur in GetAllPools and stop the program

Proof of Concept

https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/coinswap/keeper/pool.go#L50 there is no check for pool value if nil or not before calling MustUnmarshal which could lead to a panic issue in the program

func (k Keeper) GetAllPools(ctx sdk.Context) (pools []types.Pool) {
    store := ctx.KVStore(k.storeKey)
    iterator := sdk.KVStorePrefixIterator(store, []byte(types.KeyPool))
    defer iterator.Close()
    for ; iterator.Valid(); iterator.Next() {
        var pool types.Pool
        k.cdc.MustUnmarshal(iterator.Value(), &pool)
        pools = append(pools, pool)
    }
    return
}

Tools Used

Manual Review

Recommended Mitigation Steps

check if pool == nil and use call MustUnmarshal for it if it is, by a return.

Assessed type

Other

c4-pre-sort commented 1 year ago

JeffCX marked the issue as low quality report

JeffCX commented 1 year ago

var pool types.Pool is a placer holder variable here, don't think the finding valid with more proof

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Insufficient proof