-
Notifications
You must be signed in to change notification settings - Fork 271
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
validation.rs panics due to improper exp while calculating less_then window/leeway #388
Comments
Should be an easy bug to fix if someone wants to try |
Taking a crack at it. I stumbled on it while testing my implementation of JWT in Postman and lazily threw a 1 in the claims.exp to see if I had the minimum info setup and thats when it panic'd. Please let me know what you think. |
I think we could probably just check whether all the timestamp values are sane before? |
Agreed, the exp timestamp should be cast into a valid Date obj or fail (DateTime::from_timestamp). I noticed that you were using wasm bindings to get the timestamps, so I'm not sure how you would want to check the timestamp is valid without adding a new binding. My thinking was simply to fix the the panic and preventing anything smaller from overflowing it. Invalid timestamps greater than this window, ex (exp = 6, window = 5) would succeed this patch check but should still fail the expiration checks that follow: exp - options.reject_tokens_expiring_in_less_than < now - options.leeway This just seemed much simpler. I welcome your thoughts. |
Your initial patch is much simpler and works, let's go with that ^ |
e.g. claims.exp can be "1" and pass the parse check but overflow in the calculation and lead to panic. stack trace below.
thread 'tokio-runtime-worker' panicked at /Users/0xd/.cargo/registry/src/index.crates.io-6f17d22bba15001f/jsonwebtoken-9.3.0/src/validation.rs:258:16:
attempt to subtract with overflow
stack backtrace:
0: rust_begin_unwind
at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/std/src/panicking.rs:647:5
1: core::panicking::panic_fmt
at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:72:14
2: core::panicking::panic
at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:144:5
3: jsonwebtoken::validation::validate
at /Users/0xd/.cargo/registry/src/index.crates.io-6f17d22bba15001f/jsonwebtoken-9.3.0/src/validation.rs:258:16
4: jsonwebtoken::decoding::decode
at /Users/0xd/.cargo/registry/src/index.crates.io-6f17d22bba15001f/jsonwebtoken-9.3.0/src/decoding.rs:267:13
The text was updated successfully, but these errors were encountered: