Skip to content

Commit d22922c

Browse files
gh-148169: Fix webbrowser %action substitution bypass of dash-prefix check (#148170)
1 parent 8ecb6b8 commit d22922c

File tree

3 files changed

+14
-2
lines changed

3 files changed

+14
-2
lines changed

Lib/test/test_webbrowser.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,15 @@ def test_open_bad_new_parameter(self):
119119
arguments=[URL],
120120
kw=dict(new=999))
121121

122+
def test_reject_action_dash_prefixes(self):
123+
browser = self.browser_class(name=CMD_NAME)
124+
with self.assertRaises(ValueError):
125+
browser.open('%action--incognito')
126+
# new=1: action is "--new-window", so "%action" itself expands to
127+
# a dash-prefixed flag even with no dash in the original URL.
128+
with self.assertRaises(ValueError):
129+
browser.open('%action', new=1)
130+
122131

123132
class EdgeCommandTest(CommandTestMixin, unittest.TestCase):
124133

Lib/webbrowser.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,6 @@ def _invoke(self, args, remote, autoraise, url=None):
274274

275275
def open(self, url, new=0, autoraise=True):
276276
sys.audit("webbrowser.open", url)
277-
self._check_url(url)
278277
if new == 0:
279278
action = self.remote_action
280279
elif new == 1:
@@ -288,7 +287,9 @@ def open(self, url, new=0, autoraise=True):
288287
raise Error("Bad 'new' parameter to open(); "
289288
f"expected 0, 1, or 2, got {new}")
290289

291-
args = [arg.replace("%s", url).replace("%action", action)
290+
self._check_url(url.replace("%action", action))
291+
292+
args = [arg.replace("%action", action).replace("%s", url)
292293
for arg in self.remote_args]
293294
args = [arg for arg in args if arg]
294295
success = self._invoke(args, True, autoraise, url)
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
A bypass in :mod:`webbrowser` allowed URLs prefixed with ``%action`` to pass
2+
the dash-prefix safety check.

0 commit comments

Comments
 (0)