Skip to content

Commit 67136fe

Browse files
authored
Escape username parameter (#901)
* Escape username to protect against xml injection vulnerability * Add test
1 parent a3ba722 commit 67136fe

File tree

2 files changed

+17
-3
lines changed

2 files changed

+17
-3
lines changed

msal/wstrust_request.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ def send_request(
6060
return parse_response(resp.text)
6161

6262

63-
def escape_password(password):
64-
return (password.replace('&', '&').replace('"', '"')
63+
def escape_xml(s):
64+
return (s.replace('&', '&').replace('"', '"')
6565
.replace("'", ''') # the only one not provided by cgi.escape(s, True)
6666
.replace('<', '&lt;').replace('>', '&gt;'))
6767

@@ -116,7 +116,7 @@ def _build_rst(username, password, cloud_audience_urn, endpoint_address, soap_ac
116116
endpoint_address=endpoint_address,
117117
time_now=wsu_time_format(now),
118118
time_expire=wsu_time_format(now + timedelta(minutes=10)),
119-
username=username, password=escape_password(password),
119+
username=escape_xml(username), password=escape_xml(password),
120120
wst=Mex.NS["wst"] if soap_action == Mex.ACTION_13 else Mex.NS["wst2005"],
121121
applies_to=cloud_audience_urn,
122122
key_type='http://docs.oasis-open.org/ws-sx/ws-trust/200512/Bearer'

tests/test_wstrust.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
from xml.etree import ElementTree as ET
3232
import os
3333

34+
from msal.wstrust_request import _build_rst, escape_xml
3435
from msal.wstrust_response import *
3536

3637
from tests import unittest
@@ -96,3 +97,16 @@ def test_token_parsing_happy_path(self):
9697
self.assertEqual(result.get("type"), SAML_TOKEN_TYPE_V1)
9798
self.assertIn(b"<saml:Assertion", result.get("token", ""))
9899

100+
101+
class Test_WsTrustRequest(unittest.TestCase):
102+
103+
def test_escape_xml(self):
104+
self.assertEqual(escape_xml('<>&"\''), '&lt;&gt;&amp;&quot;&apos;')
105+
106+
def test_username_xml_injection_is_prevented(self):
107+
malicious = 'admin</wsse:Username><x>INJECTED'
108+
rst = _build_rst(malicious, 'pw', 'urn:x', 'https://x',
109+
'http://docs.oasis-open.org/ws-sx/ws-trust/200512/RST/Issue')
110+
self.assertEqual(rst.count('<wsse:Username>'), 1)
111+
self.assertNotIn('<x>INJECTED', rst)
112+

0 commit comments

Comments
 (0)