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

Govshuttle module does not register it messages to LegacyAminoCodec #9

Closed
c4-bot-6 opened this issue Jun 16, 2024 · 3 comments
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-29 ineligible for award 🤖_primary AI based primary recommendation 🤖_05_group AI based duplicate group recommendation unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-6
Copy link
Contributor

Lines of code

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

Vulnerability details

The GovShuttle implements the AppModuleBasic interface by implementing its methods.

Among these, the RegisterLegacyAminoCodec has an incorrect implementation (canto-main/x/govshuttle/module.go):

func (AppModuleBasic) RegisterCodec(cdc *codec.LegacyAmino) {
	types.RegisterCodec(cdc)
}

func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
	types.RegisterCodec(cdc)
}

In the (AppModuleBasic) RegisterLegacyAminoCodec function we can see that the Govshuttle module calls the types.RegisterCodec(cdc)
function that does nothing:

func RegisterCodec(cdc *codec.LegacyAmino) {
	// this line is used by starport scaffolding # 2
}

This means that the cdc.RegisterConcrete calls that other modules make, aren't made for the messages directed to the
Govshuttle module, so these types won't be properly decoded when signed with the LegacyAmino codec.

Impact

Messages to the Govshuttle module will fail when signed with the legacy Amino method

Proof of Concept

To prove this issue it is sufficient to send any message signed with the Legacy Amino method to the GovShuttle module

Tools Used

Code review

Recommended Mitigation Steps

Consider modifying the RegisterLegacyAminoCodec function as follows:

func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
    cdc.RegisterConcrete(&types.MsgLendingMarketProposal, "...")
    cdc.RegisterConcrete(&types.MsgTreasuryProposal, "...")
}

The AppModuleBasic RegisterCodec function can also be removed as it has no caller.

Assessed type

Other

@c4-bot-6 c4-bot-6 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jun 16, 2024
c4-bot-1 added a commit that referenced this issue Jun 16, 2024
@c4-bot-13 c4-bot-13 added 🤖_05_group AI based duplicate group recommendation 🤖_primary AI based primary recommendation labels Jun 20, 2024
@thebrittfactor
Copy link

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.

@C4-Staff
Copy link

CloudEllie marked the issue as duplicate of #29

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Jun 27, 2024
@c4-judge
Copy link

3docSec marked the issue as unsatisfactory:
Insufficient proof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-29 ineligible for award 🤖_primary AI based primary recommendation 🤖_05_group AI based duplicate group recommendation unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

5 participants