validation.rs panics due to improper exp while calculating less_then window/leeway
if matches!(claims.exp, TryParse::Parsed(exp) if options.validate_exp
&& exp - options.reject_tokens_expiring_in_less_than < now - options.leeway )
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
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. issue_388.patch
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 ^