From: Ben Darnell Date: Thu, 21 May 2026 20:02:48 +0000 (-0400) Subject: auth: Correctly parse check_authentication response X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=b6e21463dfec0082e479b555362da9045b64df78;p=thirdparty%2Ftornado.git auth: Correctly parse check_authentication response This previously used substring search, which is incorrect, although unlikely to be a vulnerability because there are no free-form text fields allowed in this response format. --- diff --git a/tornado/auth.py b/tornado/auth.py index 94a6c50c..31115f6f 100644 --- a/tornado/auth.py +++ b/tornado/auth.py @@ -73,6 +73,7 @@ import base64 import binascii import hashlib import hmac +import re import time import urllib.parse import uuid @@ -216,7 +217,7 @@ class OpenIdMixin: self, response: httpclient.HTTPResponse ) -> dict[str, Any]: handler = cast(RequestHandler, self) - if b"is_valid:true" not in response.body: + if re.search(rb"(?m)^is_valid:true$", response.body) is None: raise AuthError("Invalid OpenID response: %r" % response.body) # Make sure we got back at least an email from attribute exchange diff --git a/tornado/test/auth_test.py b/tornado/test/auth_test.py index 28fa7309..a411a159 100644 --- a/tornado/test/auth_test.py +++ b/tornado/test/auth_test.py @@ -41,10 +41,20 @@ class OpenIdClientLoginHandler(RequestHandler, OpenIdMixin): class OpenIdServerAuthenticateHandler(RequestHandler): + flip_flop = False + def post(self): if self.get_argument("openid.mode") != "check_authentication": raise Exception("incorrect openid.mode %r") - self.write("is_valid:true") + # Cover both orderings of the response parameters if we call this handler twice. + # (the flip_flop side effect is simpler than plumbing parameters around). + # We check both orderings to catch mistaken uses of re.match instead of re.search + # or incorrect matching of the newline characters. + if type(self).flip_flop: + self.write("is_valid:true\nns:http://specs.openid.net/auth/2.0\n") + else: + self.write("ns:http://specs.openid.net/auth/2.0\nis_valid:true\n") + type(self).flip_flop = not type(self).flip_flop class OAuth1ClientLoginHandler(RequestHandler, OAuthMixin): @@ -338,15 +348,17 @@ class AuthTest(AsyncHTTPTestCase): self.assertIn("/openid/server/authenticate?", response.headers["Location"]) def test_openid_get_user(self): - response = self.fetch( - "/openid/client/login?openid.mode=blah" - "&openid.ns.ax=http://openid.net/srv/ax/1.0" - "&openid.ax.type.email=http://axschema.org/contact/email" - "&openid.ax.value.email=foo@example.com" - ) - response.rethrow() - parsed = json_decode(response.body) - self.assertEqual(parsed["email"], "foo@example.com") + for i in range(2): + with self.subTest(i=i): + response = self.fetch( + "/openid/client/login?openid.mode=blah" + "&openid.ns.ax=http://openid.net/srv/ax/1.0" + "&openid.ax.type.email=http://axschema.org/contact/email" + "&openid.ax.value.email=foo@example.com" + ) + response.rethrow() + parsed = json_decode(response.body) + self.assertEqual(parsed["email"], "foo@example.com") def test_oauth10_redirect(self): response = self.fetch("/oauth10/client/login", follow_redirects=False)