diff --git a/constitution.py b/constitution.py index ef307616c2b0aab5267d763ec025de315903bc99..a99b2715ce53121885a13ec912fcd0c8d9576367 100644 --- a/constitution.py +++ b/constitution.py @@ -6,7 +6,7 @@ # "uvicorn", # "httpx", # "tenacity", -# "evaleval @ git+https://github.com/tommy-mor/evaleval.git@584225b43f37261b446ad04169aaddf77ca6c201", +# "evaleval @ git+https://github.com/tommy-mor/evaleval.git@e330d82a7e813e59b7e594f4c990a6c66d8fb0c2", # "rocksdict>=0.3.29", # "authlib", # "itsdangerous", @@ -4621,8 +4621,9 @@ async def index(request: Request): async def do(request: Request): form = await request.form() try: - snippet = signer.verify_snippet(form) - result = eval(snippet) + # verify_snippet binds each $slot's value as an eval local rather than + # splicing it into the source, so request data can never become code. + result = signer.verify_snippet(form).eval(globals()) if asyncio.iscoroutine(result): result = await result return result diff --git a/pyproject.toml b/pyproject.toml index 54e55b598c7a53da3f32fa5802cca3fa1db3fcb2..97bdb0c6176648ec465c0eaf8f69f4bee9c4ca6b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ name = "slug-constitution" version = "0.1.0" requires-python = ">=3.11" dependencies = [ - "evaleval @ git+https://github.com/tommy-mor/evaleval.git@584225b43f37261b446ad04169aaddf77ca6c201", + "evaleval @ git+https://github.com/tommy-mor/evaleval.git@e330d82a7e813e59b7e594f4c990a6c66d8fb0c2", "rocksdict>=0.3.29", "numpy", "httpx", diff --git a/tests/test_snippet_binding.py b/tests/test_snippet_binding.py new file mode 100644 index 0000000000000000000000000000000000000000..b7255c3e2d4be318c466adc6a6d87b164b33d5fe --- /dev/null +++ b/tests/test_snippet_binding.py @@ -0,0 +1,92 @@ +"""Signed snippets bind $slots as eval locals, never as spliced source. + +Regression coverage for a remote-code-execution class: a request value that +contains a `$slot` reference must not be able to reintroduce a live slot and +realign string quotes so that attacker bytes land in code position. Values are +bound to eval locals (via evaleval's BoundSnippet), so they can only ever be +data. These tests drive the exact path used by the POST / handler. +""" + +import constitution as c + + +def _signed(template: str) -> dict: + nonce = c.signer.generate_nonce() + sig = c.signer.sign(template, nonce) + return {"__snippet__": template, "__sig__": sig, "__nonce__": nonce} + + +def test_verify_snippet_binds_value_as_local_not_source(): + form = {**_signed("redeem('alice', $wallet_address)"), "wallet_address": "0xABC"} + bound = c.signer.verify_snippet(form) + assert "0xABC" not in bound.source # value is not spliced into the source + assert "0xABC" in bound.locals_.values() + + calls = [] + bound.eval({"redeem": lambda u, w: calls.append((u, w))}) + assert calls == [("alice", "0xABC")] + + +def test_nested_slot_payload_cannot_execute(): + # Historical RCE: wallet_address="$q" would, under string splicing, + # reintroduce $q inside a scrubbed literal; scrubbing q's payload then + # realigned quotes so __import__(...) became a live call argument. + payload = "', __import__('os').system('touch /tmp/pwned'), '" + form = { + **_signed("redeem('alice', $wallet_address)"), + "wallet_address": "$q", + "q": payload, + } + bound = c.signer.verify_snippet(form) + + calls = [] + bound.eval({"redeem": lambda *a: calls.append(a) or "ok"}) + # redeem is called exactly once, with the payload as an inert string. + assert calls == [("alice", "$q")] + + +def test_extra_fields_cannot_shadow_globals(): + # A submitted field named like a global must not leak into the eval scope + # and shadow the real handler. + form = { + **_signed("redeem('alice', $wallet_address)"), + "wallet_address": "0xABC", + "redeem": "attacker", + "os": "attacker", + } + bound = c.signer.verify_snippet(form) + sentinel = object() + called = [] + + def redeem(u, w): + called.append((u, w)) + return sentinel + + assert bound.eval({"redeem": redeem}) is sentinel + assert called == [("alice", "0xABC")] + + +def test_bad_signature_is_rejected(): + form = { + "__snippet__": "redeem('alice', $wallet_address)", + "__sig__": "not-a-real-signature", + "__nonce__": c.signer.generate_nonce(), + "wallet_address": "0xABC", + } + try: + c.signer.verify_snippet(form) + except c.SnippetExecutionError as exc: + assert exc.status_code == 403 + else: + raise AssertionError("expected SnippetExecutionError for bad signature") + + +def test_nonce_is_single_use(): + form = {**_signed("redeem('alice', $wallet_address)"), "wallet_address": "0xABC"} + c.signer.verify_snippet(form) + try: + c.signer.verify_snippet(form) + except c.SnippetExecutionError as exc: + assert exc.status_code == 403 + else: + raise AssertionError("nonce must be single-use") diff --git a/uv.lock b/uv.lock index 1873bd42292f8d7e6e25df65246ab43608768676..578136997899595752a8f66d911f891251e2816f 100644 --- a/uv.lock +++ b/uv.lock @@ -281,7 +281,7 @@ wheels = [ [[package]] name = "evaleval" version = "0.4.0" -source = { git = "https://github.com/tommy-mor/evaleval.git?rev=584225b43f37261b446ad04169aaddf77ca6c201#584225b43f37261b446ad04169aaddf77ca6c201" } +source = { git = "https://github.com/tommy-mor/evaleval.git?rev=e330d82a7e813e59b7e594f4c990a6c66d8fb0c2#e330d82a7e813e59b7e594f4c990a6c66d8fb0c2" } dependencies = [ { name = "cbor2" }, { name = "rocksdict" }, @@ -777,7 +777,7 @@ dependencies = [ [package.metadata] requires-dist = [ { name = "authlib" }, - { name = "evaleval", git = "https://github.com/tommy-mor/evaleval.git?rev=584225b43f37261b446ad04169aaddf77ca6c201" }, + { name = "evaleval", git = "https://github.com/tommy-mor/evaleval.git?rev=e330d82a7e813e59b7e594f4c990a6c66d8fb0c2" }, { name = "fastapi" }, { name = "httpx" }, { name = "hypothesis" },