mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 19:26:48 +00:00
fix(storages): the pool target check runs in linear time
The expression that checks a storage pool's target directory nested a repetition inside another, so a crafted target backtracked exponentially: "," followed by 22 dashes and "!" took 1.3 s, and every two more characters four times as long. The field allows 100 characters, so one form post could hold a worker for good (CodeQL py/redos; the page is for superusers). The new expression has no nested repetition and accepts exactly the same targets: compared with the old one on 200,000 random strings over letters, digits, "/", ".", "-", "_", ",", "!", space and newline, they agree on every string. A crafted 100-character target is now checked in microseconds. The tests pin the accepted and refused targets and time the crafted one.
This commit is contained in:
parent
940545c2c4
commit
09fbabf705
2 changed files with 37 additions and 1 deletions
|
|
@ -38,7 +38,10 @@ class AddStgPool(forms.Form):
|
|||
def clean_target(self):
|
||||
storage_type = self.cleaned_data["stg_type"]
|
||||
target = self.cleaned_data["target"]
|
||||
have_symbol = re.match(r"^[^-](\/?((?:[0-9a-zA-Z_])|(?:\.)|((?<!\/)-))*)+$", target)
|
||||
# letters, digits, "_", ".", "/" and "-" not right after "/"; not
|
||||
# starting with "-". Written without nested repetition, which
|
||||
# backtracked exponentially on crafted input.
|
||||
have_symbol = re.match(r"^[^-](?:[/0-9a-zA-Z_.]|(?<!/)-)*$", target)
|
||||
if storage_type in ["dir", "netfs"] and not have_symbol:
|
||||
raise forms.ValidationError(
|
||||
_("The target must not contain any special characters")
|
||||
|
|
|
|||
33
storages/test_pool_target.py
Normal file
33
storages/test_pool_target.py
Normal file
|
|
@ -0,0 +1,33 @@
|
|||
"""The storage pool target check runs in linear time and accepts what it did."""
|
||||
|
||||
import time
|
||||
|
||||
from django.test import SimpleTestCase
|
||||
from storages.forms import AddStgPool
|
||||
|
||||
|
||||
def target_error(target):
|
||||
form = AddStgPool({"name": "pool1", "stg_type": "dir", "target": target})
|
||||
form.is_valid()
|
||||
return "target" in form.errors
|
||||
|
||||
|
||||
class PoolTargetTests(SimpleTestCase):
|
||||
def test_accepted_and_refused_targets(self):
|
||||
for target, refused in (
|
||||
("/var/lib/libvirt/images", False),
|
||||
("/srv/iso-pool", False),
|
||||
("/a//b", False),
|
||||
("a-b.c_d/e", False),
|
||||
("/a/-b", True), # a dash right after a slash
|
||||
("-x", True),
|
||||
("/a b", True),
|
||||
):
|
||||
with self.subTest(target=target):
|
||||
self.assertEqual(target_error(target), refused)
|
||||
|
||||
def test_a_crafted_target_is_checked_at_once(self):
|
||||
# the previous expression backtracked exponentially: 22 dashes took over a second
|
||||
start = time.monotonic()
|
||||
self.assertTrue(target_error("," + "-" * 98 + "!"))
|
||||
self.assertLess(time.monotonic() - start, 1)
|
||||
Loading…
Add table
Add a link
Reference in a new issue