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

staking: enforce length limit on validator info fields #2066

Closed
plaidfinch opened this issue Feb 28, 2023 · 2 comments · Fixed by #3619
Closed

staking: enforce length limit on validator info fields #2066

plaidfinch opened this issue Feb 28, 2023 · 2 comments · Fixed by #3619
Assignees
Labels
A-node Area: System design and implementation for node software A-staking Area: Design and implementation of staking and delegation C-bug Category: a bug E-easy Effort: Easy good first issue A good issue for people without existing context on the project. help wanted Help on this issue is welcomed! security Issues or work related to security.

Comments

@plaidfinch
Copy link
Collaborator

User @what's_next?#1511 discovered that there is no length limit imposed on validator names, and uploaded the following validator definition to testnet 047-lysithea:

https://gist.github.com/plaidfinch/67b043b9569dbefcdc4db26adb80f070

As you can see, both the name and website fields contain a 29kB base64-encoded GIF, specifically this one:

barcode

We should enforce a reasonable maximum length limit for all fields in validator definitions, in consensus. We should also take this opportunity to check that any other freeform String fields that are user-controlled also have length maximums imposed.

@plaidfinch plaidfinch added C-bug Category: a bug A-node Area: System design and implementation for node software A-staking Area: Design and implementation of staking and delegation labels Feb 28, 2023
@hdevalence
Copy link
Member

Length limits are a good idea. In addition, once we implement fees, we should also have a per-byte fee for blockspace usage.

@plaidfinch plaidfinch moved this to Testnet 49: Pasiphae in Testnets Mar 10, 2023
@plaidfinch plaidfinch self-assigned this Mar 10, 2023
@hdevalence hdevalence moved this from Testnet 49: Pasiphae to Next in Testnets Mar 16, 2023
@hdevalence
Copy link
Member

Let's bound the name, website, etc fields at 140 characters and the description at 280 characters.

Let's also scope this issue just to fixing the validator definitions to have bounds. We can do a general survey of other messages at a later point.

@hdevalence hdevalence added E-easy Effort: Easy help wanted Help on this issue is welcomed! good first issue A good issue for people without existing context on the project. security Issues or work related to security. labels Mar 18, 2023
@plaidfinch plaidfinch moved this from Next to Testnet 53: Himalia in Testnets May 5, 2023
@erwanor erwanor moved this from Testnet 53: Himalia to Next in Testnets May 19, 2023
@conorsch conorsch assigned conorsch and unassigned plaidfinch Jul 20, 2023
@aubrika aubrika moved this from Next to Testnet 64: Titan in Testnets Oct 18, 2023
@aubrika aubrika added this to Penumbra Oct 30, 2023
@github-project-automation github-project-automation bot moved this to 🗄️ Backlog in Penumbra Oct 30, 2023
@aubrika aubrika moved this from 🗄️ Backlog to 📝 Todo in Penumbra Oct 30, 2023
@aubrika aubrika moved this from 📝 Todo to 🗄️ Backlog in Penumbra Nov 16, 2023
@erwanor erwanor moved this from Testnet 64: Titan to Next in Testnets Nov 29, 2023
@erwanor erwanor moved this from Next to Testnet 65: Deimos in Testnets Jan 8, 2024
@erwanor erwanor self-assigned this Jan 16, 2024
@erwanor erwanor moved this from 🗄️ Backlog to Todo in Penumbra Jan 16, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Penumbra Jan 17, 2024
@github-project-automation github-project-automation bot moved this from Testnet 65: Deimos to Testnet 63: Rhea (Web Wallet) in Testnets Jan 17, 2024
@erwanor erwanor changed the title Enforce length limit on validator info fields staking: enforce length limit on validator info fields Jan 18, 2024
@erwanor erwanor moved this from Testnet 63: Rhea (Web Wallet) to Testnet 65: Deimos in Testnets Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-node Area: System design and implementation for node software A-staking Area: Design and implementation of staking and delegation C-bug Category: a bug E-easy Effort: Easy good first issue A good issue for people without existing context on the project. help wanted Help on this issue is welcomed! security Issues or work related to security.
Projects
Archived in project
Status: Testnet 65: Deimos
Development

Successfully merging a pull request may close this issue.

4 participants