search

code-423n4 / 2024-05-canto-findings

0 stars 0 forks source link

Govshuttle module does not register its transaction MsgServer #5

Open c4-bot-5 opened 3 months ago

c4-bot-5 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-05-canto/blob/d1d51b2293d4689f467b8b1c82bba84f8f7ea008/canto-main/x/govshuttle/module.go#L127

Vulnerability details

The x/govshuttle module in canto-main defines and handles two messages that can be emitted by a governance proposal:

However, because the module only registers the QueryServer (and not its MsgServer) in its RegisterServices function, causing no message to be routed to its message server:

func (am AppModule) RegisterServices(cfg module.Configurator) {
    types.RegisterQueryServer(cfg.QueryServer(), am.keeper)
}

If we compare this with another module that can handle messages, for example CSR, we see that this is the place for registering the MsgServer where transactional messages are routed to:

func (am AppModule) RegisterServices(cfg module.Configurator) {
    types.RegisterMsgServer(cfg.MsgServer(), am.keeper)
    types.RegisterQueryServer(cfg.QueryServer(), am.keeper)
}

Impact

Successful governance actions that include a LendingMarketProposal or TreasuryProposal will fail to execute because no handler is provided for them.

Proof of Concept

To reproduce the issue it is sufficient to create and approve a proposal among the affected ones.

Tools Used

Code review

Recommended Mitigation Steps

Consider adding a RegisterMsgServer call in the x/govshuttle RegisterService callback.

Assessed type

Other

thebrittfactor commented 2 months ago

Warden will be acting as the judge for this audit and therefore, has agreed to forfeit their submissions and will not be eligible for awards for this audit.

dudong2 commented 2 months ago
c4-judge commented 2 months ago

3docSec marked the issue as satisfactory

c4-judge commented 2 months ago

3docSec marked the issue as selected for report

thebrittfactor commented 2 months ago

For transparency, staff have added the appropriate sponsor label, per discord confirmation and this comment from the sponsor team.