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

ContractName::try_from() allows invalid ContractName to be created #4727

Open
jbencin opened this issue Apr 29, 2024 · 0 comments
Open

ContractName::try_from() allows invalid ContractName to be created #4727

jbencin opened this issue Apr 29, 2024 · 0 comments

Comments

@jbencin
Copy link
Contributor

jbencin commented Apr 29, 2024

Description

The ContractName struct is implemented using a macro, like this:

guarded_string!(
    ContractName,
    "ContractName",
    CONTRACT_NAME_REGEX,
    MAX_STRING_LEN,
    RuntimeErrorType,
    RuntimeErrorType::BadNameValue
);

MAX_STRING_LEN is 128, so it's possible to construct a ContractName with up to 128 characters. However, when we implement StacksMessageCodec for ContractName:

    fn consensus_serialize<W: Write>(&self, fd: &mut W) -> Result<(), codec_error> {
        if self.as_bytes().len() < CONTRACT_MIN_NAME_LENGTH as usize
            || self.as_bytes().len() > CONTRACT_MAX_NAME_LENGTH as usize
        {
            // Error!
        }
        // ...
    }

CONTRACT_MAX_NAME_LENGTH is 40, much smaller than MAX_STRING_LEN. This means that ContractName::try_from() allows us to create structs that can't be serialized

Proposed solutions

  • Change the macro to use CONTRACT_MAX_NAME_LENGTH instead of MAX_STRING_LEN. This won't actually fix the problem, and will likely expose more invalid ContractNames that were never being serialized, but it will make invalid ContractNames easy to find and fix. Where it fails, we can replace ContractName::try_from() with ContractName::from_truncated() or ContractName::try_from_truncated()
  • Change CONTRACT_MAX_NAME_LENGTH to match MAX_STRING_LEN. Probably not what we want to do, but would fix the issue immediately

Related issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Status: 💻 In Progress
Development

No branches or pull requests

2 participants