Split out of #39 rather than folded into 3.13.92, so the JWT release stayed scoped to the algorithm and nbf contract.
validToken compares exp numerically without checking that it is a number. A token whose exp is a string, null, an object, or an array does not fail as a malformed token -- it goes through JavaScript's coercion rules, and the outcome depends on what the value coerces to:
exp: "9999999999" coerces to a number and the comparison happens to work
exp: null coerces to 0, so the token reads as long expired (fails closed -- harmless)
exp: {} or exp: [] produce NaN, and every comparison with NaN is false
That last case is the one to check: if the expiry test is written as now > exp, a NaN makes it false, which reads as "not expired". A token carrying exp: {} would then validate on the expiry check. Whether that is reachable depends on the exact comparison and the surrounding guards, so this needs verifying against the real code before being called a security issue -- I have not confirmed it is exploitable, only that the type is unchecked.
Wanted: reject a non-numeric exp as a malformed token, the same way an unsupported algorithm now throws instead of silently degrading. Same treatment for nbf, which 3.13.92 just added and which has the identical shape.
Parity: check all four frameworks before fixing. Python raises on int()/float() of a non-numeric, so it likely already fails loudly; PHP and Ruby have their own coercion rules and need their own look. Fix once at parity, with positive and negative lock-in tests per framework, per the usual rule.
Split out of #39 rather than folded into 3.13.92, so the JWT release stayed scoped to the algorithm and
nbfcontract.validTokencomparesexpnumerically without checking that it is a number. A token whoseexpis a string,null, an object, or an array does not fail as a malformed token -- it goes through JavaScript's coercion rules, and the outcome depends on what the value coerces to:exp: "9999999999"coerces to a number and the comparison happens to workexp: nullcoerces to0, so the token reads as long expired (fails closed -- harmless)exp: {}orexp: []produceNaN, and every comparison withNaNis falseThat last case is the one to check: if the expiry test is written as
now > exp, aNaNmakes it false, which reads as "not expired". A token carryingexp: {}would then validate on the expiry check. Whether that is reachable depends on the exact comparison and the surrounding guards, so this needs verifying against the real code before being called a security issue -- I have not confirmed it is exploitable, only that the type is unchecked.Wanted: reject a non-numeric
expas a malformed token, the same way an unsupported algorithm now throws instead of silently degrading. Same treatment fornbf, which 3.13.92 just added and which has the identical shape.Parity: check all four frameworks before fixing. Python raises on
int()/float()of a non-numeric, so it likely already fails loudly; PHP and Ruby have their own coercion rules and need their own look. Fix once at parity, with positive and negative lock-in tests per framework, per the usual rule.