Skip to content

Commit 065c08d

Browse files
chipshortchipshort
authored
Merge pull request #2291 from CosmWasm/co/audit-fixes
Security fixes on main
2 parents 008e4f5 + 63537af commit 065c08d

File tree

4 files changed

+77
-22
lines changed

4 files changed

+77
-22
lines changed

‎tests/integration/ibc_integration_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,10 +397,10 @@ func TestIBCAsyncAck(t *testing.T) {
397397
path = wasmibctesting.NewWasmPath(chainA, chainB)
398398
)
399399
path.EndpointA.ChannelConfig = &ibctesting.ChannelConfig{
400-
PortID: sourcePortID, Version: "ibc-reflect-v1", Order: channeltypes.UNORDERED,
400+
PortID: sourcePortID, Version: "ibc-reflect-v1", Order: channeltypes.ORDERED,
401401
}
402402
path.EndpointB.ChannelConfig = &ibctesting.ChannelConfig{
403-
PortID: counterpartPortID, Version: "ibc-reflect-v1", Order: channeltypes.UNORDERED,
403+
PortID: counterpartPortID, Version: "ibc-reflect-v1", Order: channeltypes.ORDERED,
404404
}
405405

406406
coord.SetupConnections(&path.Path)

‎x/wasm/keeper/msg_dispatcher.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,18 @@ func (d MessageDispatcher) dispatchMsgWithGasLimit(ctx sdk.Context, contractAddr
6868
// catch out of gas panic and just charge the entire gas limit
6969
defer func() {
7070
if r := recover(); r != nil {
71-
// if it's not an OutOfGas error, raise it again
72-
if _, ok := r.(storetypes.ErrorOutOfGas); !ok {
71+
if _, ok := r.(storetypes.ErrorOutOfGas); ok {
72+
// consume the gas limit for the submessage and turn panic into error
73+
ctx.GasMeter().ConsumeGas(gasLimit, "Sub-Message OutOfGas panic")
74+
err = errorsmod.Wrap(sdkerrors.ErrOutOfGas, "SubMsg hit gas limit")
75+
} else {
76+
// if it's not an ErrorOutOfGas, consume the gas used in the sub-context and raise it again
77+
spent := subCtx.GasMeter().GasConsumed()
78+
ctx.GasMeter().ConsumeGas(spent, "From limited Sub-Message")
7379
// log it to get the original stack trace somewhere (as panic(r) keeps message but stacktrace to here
7480
moduleLogger(ctx).Info("SubMsg rethrowing panic: %#v", r)
7581
panic(r)
7682
}
77-
ctx.GasMeter().ConsumeGas(gasLimit, "Sub-Message OutOfGas panic")
78-
err = errorsmod.Wrap(sdkerrors.ErrOutOfGas, "SubMsg hit gas limit")
7983
}
8084
}()
8185
events, data, msgResponses, err = d.messenger.DispatchMsg(subCtx, contractAddr, ibcPort, msg)

‎x/wasm/keeper/relay.go

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,24 +29,40 @@ func (k Keeper) OnOpenChannel(
2929
msg wasmvmtypes.IBCChannelOpenMsg,
3030
) (string, error) {
3131
defer telemetry.MeasureSince(time.Now(), "wasm", "contract", "ibc-open-channel")
32-
_, codeInfo, prefixStore, err := k.contractInstance(ctx, contractAddr)
32+
contractInfo, codeInfo, prefixStore, err := k.contractInstance(ctx, contractAddr)
3333
if err != nil {
3434
return "", err
3535
}
3636

37+
sdkCtx := sdk.UnwrapSDKContext(ctx)
38+
sdkCtx, discount := k.checkDiscountEligibility(sdkCtx, codeInfo.CodeHash, k.IsPinnedCode(ctx, contractInfo.CodeID))
39+
setupCost := k.gasRegister.SetupContractCost(discount, msg.ExpectedJSONSize())
40+
sdkCtx.GasMeter().ConsumeGas(setupCost, "Loading CosmWasm module: ibc-open-channel")
41+
3742
env := types.NewEnv(ctx, k.txHash, contractAddr)
3843
querier := k.newQueryHandler(ctx, contractAddr)
3944

4045
gasLeft := k.runtimeGasForContract(ctx)
4146
res, gasUsed, execErr := k.wasmVM.IBCChannelOpen(codeInfo.CodeHash, env, msg, prefixStore, cosmwasmAPI, querier, ctx.GasMeter(), gasLeft, costJSONDeserialization)
4247
k.consumeRuntimeGas(ctx, gasUsed)
48+
// check if contract panicked / VM failed
4349
if execErr != nil {
4450
return "", errorsmod.Wrap(types.ErrExecuteFailed, execErr.Error())
4551
}
46-
if res != nil && res.Ok != nil {
47-
return res.Ok.Version, nil
52+
if res == nil {
53+
// If this gets executed, that's a bug in wasmvm
54+
return "", errorsmod.Wrap(types.ErrVMError, "internal wasmvm error")
55+
}
56+
// check contract result
57+
if res.Err != "" {
58+
return "", types.MarkErrorDeterministic(errorsmod.Wrap(types.ErrExecuteFailed, res.Err))
59+
}
60+
if res.Ok == nil {
61+
// a nil "ok" value is a valid response and means the contract accepts the incoming channel version
62+
// see https://docs.rs/cosmwasm-std/2.2.2/cosmwasm_std/type.IbcChannelOpenResponse.html
63+
return "", nil
4864
}
49-
return "", nil
65+
return res.Ok.Version, nil
5066
}
5167

5268
// OnConnectChannel calls the contract to let it know the IBC channel was established.
@@ -67,6 +83,11 @@ func (k Keeper) OnConnectChannel(
6783
return err
6884
}
6985

86+
sdkCtx := sdk.UnwrapSDKContext(ctx)
87+
sdkCtx, discount := k.checkDiscountEligibility(sdkCtx, codeInfo.CodeHash, k.IsPinnedCode(ctx, contractInfo.CodeID))
88+
setupCost := k.gasRegister.SetupContractCost(discount, msg.ExpectedJSONSize())
89+
sdkCtx.GasMeter().ConsumeGas(setupCost, "Loading CosmWasm module: ibc-connect-channel")
90+
7091
env := types.NewEnv(ctx, k.txHash, contractAddr)
7192
querier := k.newQueryHandler(ctx, contractAddr)
7293

@@ -105,6 +126,11 @@ func (k Keeper) OnCloseChannel(
105126
return err
106127
}
107128

129+
sdkCtx := sdk.UnwrapSDKContext(ctx)
130+
sdkCtx, discount := k.checkDiscountEligibility(sdkCtx, codeInfo.CodeHash, k.IsPinnedCode(ctx, contractInfo.CodeID))
131+
setupCost := k.gasRegister.SetupContractCost(discount, msg.ExpectedJSONSize())
132+
sdkCtx.GasMeter().ConsumeGas(setupCost, "Loading CosmWasm module: ibc-close-channel")
133+
108134
params := types.NewEnv(ctx, k.txHash, contractAddr)
109135
querier := k.newQueryHandler(ctx, contractAddr)
110136

@@ -142,6 +168,11 @@ func (k Keeper) OnRecvPacket(
142168
return nil, err
143169
}
144170

171+
sdkCtx := sdk.UnwrapSDKContext(ctx)
172+
sdkCtx, discount := k.checkDiscountEligibility(sdkCtx, codeInfo.CodeHash, k.IsPinnedCode(ctx, contractInfo.CodeID))
173+
setupCost := k.gasRegister.SetupContractCost(discount, msg.ExpectedJSONSize())
174+
sdkCtx.GasMeter().ConsumeGas(setupCost, "Loading CosmWasm module: ibc-recv-packet")
175+
145176
env := types.NewEnv(ctx, k.txHash, contractAddr)
146177
querier := k.newQueryHandler(ctx, contractAddr)
147178

@@ -215,6 +246,11 @@ func (k Keeper) OnAckPacket(
215246
return err
216247
}
217248

249+
sdkCtx := sdk.UnwrapSDKContext(ctx)
250+
sdkCtx, discount := k.checkDiscountEligibility(sdkCtx, codeInfo.CodeHash, k.IsPinnedCode(ctx, contractInfo.CodeID))
251+
setupCost := k.gasRegister.SetupContractCost(discount, msg.ExpectedJSONSize())
252+
sdkCtx.GasMeter().ConsumeGas(setupCost, "Loading CosmWasm module: ibc-ack-packet")
253+
218254
env := types.NewEnv(ctx, k.txHash, contractAddr)
219255
querier := k.newQueryHandler(ctx, contractAddr)
220256

@@ -250,6 +286,11 @@ func (k Keeper) OnTimeoutPacket(
250286
return err
251287
}
252288

289+
sdkCtx := sdk.UnwrapSDKContext(ctx)
290+
sdkCtx, discount := k.checkDiscountEligibility(sdkCtx, codeInfo.CodeHash, k.IsPinnedCode(ctx, contractInfo.CodeID))
291+
setupCost := k.gasRegister.SetupContractCost(discount, msg.ExpectedJSONSize())
292+
sdkCtx.GasMeter().ConsumeGas(setupCost, "Loading CosmWasm module: ibc-timeout-packet")
293+
253294
env := types.NewEnv(ctx, k.txHash, contractAddr)
254295
querier := k.newQueryHandler(ctx, contractAddr)
255296

@@ -284,6 +325,11 @@ func (k Keeper) IBCSourceCallback(
284325
return err
285326
}
286327

328+
sdkCtx := sdk.UnwrapSDKContext(ctx)
329+
sdkCtx, discount := k.checkDiscountEligibility(sdkCtx, codeInfo.CodeHash, k.IsPinnedCode(ctx, contractInfo.CodeID))
330+
setupCost := k.gasRegister.SetupContractCost(discount, msg.ExpectedJSONSize())
331+
sdkCtx.GasMeter().ConsumeGas(setupCost, "Loading CosmWasm module: ibc-source-chain-callback")
332+
287333
env := types.NewEnv(ctx, k.txHash, contractAddr)
288334
querier := k.newQueryHandler(ctx, contractAddr)
289335

@@ -318,6 +364,11 @@ func (k Keeper) IBCDestinationCallback(
318364
return err
319365
}
320366

367+
sdkCtx := sdk.UnwrapSDKContext(ctx)
368+
sdkCtx, discount := k.checkDiscountEligibility(sdkCtx, codeInfo.CodeHash, k.IsPinnedCode(ctx, contractInfo.CodeID))
369+
setupCost := k.gasRegister.SetupContractCost(discount, msg.ExpectedJSONSize())
370+
sdkCtx.GasMeter().ConsumeGas(setupCost, "Loading CosmWasm module: ibc-destination-chain-callback")
371+
321372
env := types.NewEnv(ctx, k.txHash, contractAddr)
322373
querier := k.newQueryHandler(ctx, contractAddr)
323374

‎x/wasm/keeper/relay_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ func TestOnOpenChannel(t *testing.T) {
8383
}
8484
require.NoError(t, err)
8585
// verify gas consumed
86-
const storageCosts = storetypes.Gas(3101)
87-
assert.Equal(t, spec.expGas, ctx.GasMeter().GasConsumed()-before-storageCosts)
86+
const storageCosts = storetypes.Gas(4101)
87+
assert.Equal(t, spec.expGas, ctx.GasMeter().GasConsumed()-before-storageCosts-types.DefaultInstanceCost)
8888
})
8989
}
9090
}
@@ -189,8 +189,8 @@ func TestOnConnectChannel(t *testing.T) {
189189
}
190190
require.NoError(t, err)
191191
// verify gas consumed
192-
const storageCosts = storetypes.Gas(3101)
193-
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts)
192+
const storageCosts = storetypes.Gas(4101)
193+
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts-types.DefaultInstanceCost)
194194
// verify msgs dispatched
195195
require.Len(t, *capturedMsgs, len(spec.contractResp.Messages))
196196
for i, m := range spec.contractResp.Messages {
@@ -299,8 +299,8 @@ func TestOnCloseChannel(t *testing.T) {
299299
}
300300
require.NoError(t, err)
301301
// verify gas consumed
302-
const storageCosts = storetypes.Gas(3101)
303-
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts)
302+
const storageCosts = storetypes.Gas(4101)
303+
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts-types.DefaultInstanceCost)
304304
// verify msgs dispatched
305305
require.Len(t, *capturedMsgs, len(spec.contractResp.Messages))
306306
for i, m := range spec.contractResp.Messages {
@@ -495,8 +495,8 @@ func TestOnRecvPacket(t *testing.T) {
495495
}
496496

497497
// verify gas consumed
498-
const storageCosts = storetypes.Gas(3101)
499-
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts)
498+
const storageCosts = storetypes.Gas(4101)
499+
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts-types.DefaultInstanceCost)
500500

501501
// verify msgs dispatched on success/ err response
502502
if spec.contractResp.Err != "" {
@@ -606,8 +606,8 @@ func TestOnAckPacket(t *testing.T) {
606606
}
607607
require.NoError(t, err)
608608
// verify gas consumed
609-
const storageCosts = storetypes.Gas(3101)
610-
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts)
609+
const storageCosts = storetypes.Gas(4101)
610+
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts-types.DefaultInstanceCost)
611611
// verify msgs dispatched
612612
require.Len(t, *capturedMsgs, len(spec.contractResp.Messages))
613613
for i, m := range spec.contractResp.Messages {
@@ -726,8 +726,8 @@ func TestOnTimeoutPacket(t *testing.T) {
726726
}
727727
require.NoError(t, err)
728728
// verify gas consumed
729-
const storageCosts = storetypes.Gas(3101)
730-
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts)
729+
const storageCosts = storetypes.Gas(4101)
730+
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts-types.DefaultInstanceCost)
731731
// verify msgs dispatched
732732
require.Len(t, *capturedMsgs, len(spec.contractResp.Messages))
733733
for i, m := range spec.contractResp.Messages {

0 commit comments

Comments
 (0)