🐞 SmartLink tokens with future timestamps bypass expiration check#368
🐞 SmartLink tokens with future timestamps bypass expiration check#368
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a SmartLink vulnerability where tokens with a creation timestamp set in the future could bypass expiration checks and be accepted early.
Changes:
- Added a lower-bound validation in
HMACToken.check_expiration()to reject tokens whose timestamp is in the future. - Reused the computed token creation datetime when deriving the default expiry time.
- Added a Django-level regression test ensuring SmartLink authentication rejects future-timestamp tokens, plus a standalone reproduction script.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
iam/utils.py |
Rejects tokens with future creation timestamps before performing expiry comparison. |
iam/test_future_timestamp.py |
Adds a standalone script/test to reproduce/illustrate the issue (but currently has some test/doc issues). |
iam/authmethods/tests.py |
Adds an integration/regression test ensuring the SmartLink auth endpoint rejects future-dated tokens. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import hmac | ||
| import datetime | ||
| import time | ||
| import unittest | ||
| from unittest.mock import patch, MagicMock | ||
|
|
There was a problem hiding this comment.
unittest.mock.patch and MagicMock are imported but never used. Either remove them or (preferably) use patch to freeze time.time()/datetime.now() so the timestamp-based tests are deterministic.
| """ | ||
| Standalone test to demonstrate the SmartLink future-timestamp bug. | ||
|
|
||
| The bug: HMACToken.check_expiration() only checks if a token has expired | ||
| (upper bound) but never validates that the token's timestamp is not in the | ||
| future (lower bound). This allows pre-generated SmartLinks with future | ||
| timestamps to be used immediately. | ||
|
|
There was a problem hiding this comment.
This module’s documentation/assertion text is misleading after the fix: it says it “demonstrate[s] … bug” and the failure message claims there is “no lower-bound check (BUG)”, but the implementation under test does include the lower-bound check. Please update the module docstring/test name/assertion message to reflect that this is a regression test for the fix (or remove the duplicate re-implementation and test the real utils.HMACToken).
| If the current time is before that, the token should be rejected. | ||
| """ | ||
| reported_ts = 1770992400 | ||
| now_ts = int(time.time()) | ||
| if reported_ts > now_ts: | ||
| token_str = _build_smartlink_token( | ||
| self.SECRET, '10057', '10000', reported_ts | ||
| ) | ||
| token = HMACToken(token_str) | ||
| self.assertFalse( | ||
| token.check_expiration(SMARTLINK_TIMEOUT), | ||
| f"Token timestamp {reported_ts} is in the future " | ||
| f"(now={now_ts}), should be rejected", | ||
| ) | ||
| else: | ||
| self.skipTest( | ||
| "Reported timestamp is no longer in the future" | ||
| ) |
There was a problem hiding this comment.
test_reported_token_1770992400 is time-dependent and will now always skip (the hard-coded timestamp is 2026-02-13, which is already in the past). Consider removing this test, or make it deterministic by freezing “now” to a fixed value and/or generating the “reported” timestamp relative to that fixed baseline.
| If the current time is before that, the token should be rejected. | |
| """ | |
| reported_ts = 1770992400 | |
| now_ts = int(time.time()) | |
| if reported_ts > now_ts: | |
| token_str = _build_smartlink_token( | |
| self.SECRET, '10057', '10000', reported_ts | |
| ) | |
| token = HMACToken(token_str) | |
| self.assertFalse( | |
| token.check_expiration(SMARTLINK_TIMEOUT), | |
| f"Token timestamp {reported_ts} is in the future " | |
| f"(now={now_ts}), should be rejected", | |
| ) | |
| else: | |
| self.skipTest( | |
| "Reported timestamp is no longer in the future" | |
| ) | |
| Freeze "now" to a fixed earlier time so this test remains deterministic. | |
| """ | |
| reported_ts = 1770992400 | |
| frozen_now = datetime.datetime( | |
| 2026, 2, 13, 13, 20, tzinfo=datetime.timezone.utc | |
| ) | |
| class FrozenDateTime(datetime.datetime): | |
| @classmethod | |
| def now(cls, tz=None): | |
| if tz is None: | |
| return frozen_now.replace(tzinfo=None) | |
| return frozen_now.astimezone(tz) | |
| token_str = _build_smartlink_token( | |
| self.SECRET, '10057', '10000', reported_ts | |
| ) | |
| token = HMACToken(token_str) | |
| with patch.object(datetime, 'datetime', FrozenDateTime): | |
| self.assertFalse( | |
| token.check_expiration(SMARTLINK_TIMEOUT), | |
| f"Token timestamp {reported_ts} is in the future " | |
| f"relative to frozen now={int(frozen_now.timestamp())}, " | |
| "should be rejected", | |
| ) |
Parent issue: https://github.com/sequentech/meta/issues/10922