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

Generation's Deserialize impl should guard against too-large values #6865

Open
jgallagher opened this issue Oct 14, 2024 · 1 comment
Open

Comments

@jgallagher
Copy link
Contributor

Generation currently has a derived Deserialize impl:

/// Generation numbers stored in the database, used for optimistic concurrency
/// control
// Because generation numbers are stored in the database, we represent them as
// i64.
#[derive(
Copy,
Clone,
Debug,
Deserialize,
Eq,
Hash,
JsonSchema,
Ord,
PartialEq,
PartialOrd,
Serialize,
)]
pub struct Generation(u64);

However, it has a note that the values should fit in an i64, and we validate that in next():

assert!(next_gen <= i64::MAX as u64);

There is no equivalent validation performed when deserializing, so I believe we could get an API request with a Generation in the range (i64::MAX, u64::MAX] and bypass the checks. I strongly suspect this doesn't really matter in practice today; Generation only appears in internal APIs, and we have no way for well-behaved clients to construct a Generation in that range to send. But it's still technically wrong and could lead to weird issues if we somehow did have a client pass a bad value, hence this issue.

@sunshowers
Copy link
Contributor

Aha yes, I was thinking of this while falling asleep last night haha :) will fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants