🛡️ Lack of Storage Address Validation in Law-Stone Instantiation

[ccamel]

Note

Severity: Low
target: v5.0.0 - Commit: cde785fbd2dad71608d53f8524e0ef8c8f8178af
Ref: OKP4 CosmWasm Audit Report v1.0 - 02-05-2024 - BlockApex

Description

The instantiation process of the law-Stone does not include validation for the storage_address provided in the InstantiateMsg.This will result in a transaction failure.

Recommendation

Implement proper validation checks in the instantiate function to ensure the storage_address is well-formed and authorized for the intended operational context.

[amimart]

I don't see any benefits in adding validation on the provided storage address, it would still end up in a transaction failure, and with the downside of causing more cost..

I'm in favour of closing this one, let me know what you think @ccamel @bdeneux

[bdeneux]

@amimart Absolutely, the verification is indeed indirectly handled when the instantiate function attempts to store the object. Therefore, an additional check might not be necessary.

Otherwise, regarding the storage_address in msg.rs, it is currently defined as a String. It could be beneficial to change its type to Addr to leverage the built-in validation provided by the JSON serde (I think it's possible).

[ccamel]

@bdeneux From what I see from: https://docs.rs/cosmwasm-std/latest/cosmwasm_std/struct.Addr.html

Addr must not be used in messages sent by the user because this would result in unvalidated instances.

[ccamel]

I would also be in favour of closing this one for all the reasons already mentioned.

[amimart]

If we all agree, let's close this one :)