From c8c83d06352e646bf8019728b21340ad5f086631 Mon Sep 17 00:00:00 2001 From: Ron Klinkien Date: Mon, 10 Aug 2026 11:50:32 +0200 Subject: [PATCH 1/2] security: validate JWT exp claim to prevent client DoS _token_expires_soon() called int(exp) on the 'exp' claim from an unverified, server-controlled JWT payload with no type guard. A hostile or MITM server returning a non-numeric exp (string -> ValueError, or a non-empty container -> TypeError) crashed every _run_request call before any HTTP request. The raw exception is not a GarminConnect* subclass, and the poisoned token is persisted by dump()/load(), so a single crafted response makes the client unusable across restarts (report 3997). Coerce exp with float() inside try/except, returning False (treat as not expiring soon) when it cannot be parsed. This also supports legitimate numeric-string exp values. Adds parametrized tests covering all PoC payloads. --- garminconnect/client.py | 10 ++++++++-- tests/test_garmin_unit.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/garminconnect/client.py b/garminconnect/client.py index 55c92b9..6c74a89 100644 --- a/garminconnect/client.py +++ b/garminconnect/client.py @@ -1314,8 +1314,14 @@ def _token_expires_soon(self) -> bool: payload = _decode_jwt_payload(str(token)) if not payload: return False - exp = payload.get("exp") - return bool(exp and time.time() > int(exp) - 900) + # 'exp' is a server-controlled claim from an unverified JWT payload, so + # coerce it defensively: a non-numeric string or container must not + # raise (ValueError/TypeError) and take down every request. + try: + exp = float(payload.get("exp")) # type: ignore[arg-type] + except (TypeError, ValueError): + return False + return time.time() > exp - 900 def _refresh_session(self) -> None: """Refresh auth — DI token refresh or legacy JWT_WEB CAS refresh.""" diff --git a/tests/test_garmin_unit.py b/tests/test_garmin_unit.py index 29c19c8..762d9bf 100644 --- a/tests/test_garmin_unit.py +++ b/tests/test_garmin_unit.py @@ -1740,6 +1740,36 @@ def test_token_expires_soon_falls_back_to_jwt_web(self): c.jwt_web = token assert c._token_expires_soon() is True + def test_token_expires_soon_accepts_numeric_string_exp(self): + # Some tokens encode 'exp' as a numeric string; it must still parse. + c = client_mod.Client(verify_login=False) + token = _make_jwt( + {"alg": "RS256"}, {"exp": str(int(time.time()) + 60)} + ) + c.di_token = token + assert c._token_expires_soon() is True + + @pytest.mark.parametrize( + "exp", + [ + "not-a-number", # non-numeric string -> would raise ValueError + {"a": 1}, # non-empty dict -> would raise TypeError + [1], # non-empty list -> would raise TypeError + None, # missing / null + "", # empty string + {}, # empty dict + [], # empty list + ], + ) + def test_token_expires_soon_survives_malformed_exp(self, exp: Any): + # A hostile/MITM server can return an access token whose unverified + # 'exp' claim is non-numeric. int()/float() coercion must not raise and + # take down every subsequent request. Malformed 'exp' + # is treated as "not expiring soon" so the client stays usable. + c = client_mod.Client(verify_login=False) + c.di_token = _make_jwt({"alg": "RS256"}, {"exp": exp}) + assert c._token_expires_soon() is False + # --------------------------------------------------------------------------- # update_workout (in-place PUT) From bc173b4cdba2e9b79eabf6531a7fa5b7ae73e8c7 Mon Sep 17 00:00:00 2001 From: Ron Klinkien Date: Mon, 10 Aug 2026 17:22:27 +0200 Subject: [PATCH 2/2] security: reject bool, non-finite and overflowing JWT exp values Addresses CodeRabbit review on PR #408: float() coercion still accepted booleans (True -> 1.0, always 'expiring soon'), non-finite strings like 'inf' (repeated refresh attempts), and raised uncaught OverflowError for huge JSON integers -- the same crash-before-every-request DoS class the original fix targeted. --- garminconnect/client.py | 15 +++++++++++---- tests/test_garmin_unit.py | 13 ++++++++++--- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/garminconnect/client.py b/garminconnect/client.py index 6c74a89..a08084d 100644 --- a/garminconnect/client.py +++ b/garminconnect/client.py @@ -13,6 +13,7 @@ import http.cookiejar import json import logging +import math import os import random import re @@ -1315,11 +1316,17 @@ def _token_expires_soon(self) -> bool: if not payload: return False # 'exp' is a server-controlled claim from an unverified JWT payload, so - # coerce it defensively: a non-numeric string or container must not - # raise (ValueError/TypeError) and take down every request. + # coerce it defensively: a non-numeric string, container, boolean, + # non-finite or overflowing value must not raise (TypeError/ValueError/ + # OverflowError) and take down every request. + raw_exp = payload.get("exp") + if isinstance(raw_exp, bool): + return False try: - exp = float(payload.get("exp")) # type: ignore[arg-type] - except (TypeError, ValueError): + exp = float(raw_exp) # type: ignore[arg-type] + except (TypeError, ValueError, OverflowError): + return False + if not math.isfinite(exp): return False return time.time() > exp - 900 diff --git a/tests/test_garmin_unit.py b/tests/test_garmin_unit.py index 762d9bf..7a1ac85 100644 --- a/tests/test_garmin_unit.py +++ b/tests/test_garmin_unit.py @@ -1759,13 +1759,20 @@ def test_token_expires_soon_accepts_numeric_string_exp(self): "", # empty string {}, # empty dict [], # empty list + True, # bool is an int subclass; must be rejected + False, + "inf", # non-finite string -> parses, must be rejected + "-inf", + "nan", + 10**400, # huge JSON int -> would raise OverflowError ], ) def test_token_expires_soon_survives_malformed_exp(self, exp: Any): # A hostile/MITM server can return an access token whose unverified - # 'exp' claim is non-numeric. int()/float() coercion must not raise and - # take down every subsequent request. Malformed 'exp' - # is treated as "not expiring soon" so the client stays usable. + # 'exp' claim is non-numeric, boolean, non-finite or overflowing. + # int()/float() coercion must not raise and take down every + # subsequent request. Malformed 'exp' is treated as "not expiring + # soon" so the client stays usable. c = client_mod.Client(verify_login=False) c.di_token = _make_jwt({"alg": "RS256"}, {"exp": exp}) assert c._token_expires_soon() is False