Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions tools/pwnshop/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ dependencies = [
"click",
"jinja2",
"pyastyle",
"pyyaml",
"rich",
]

Expand Down
3 changes: 2 additions & 1 deletion tools/pwnshop/src/pwnshop/commands/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@
@click.argument("command", nargs=-1, default=("/bin/bash",))
def run_command(challenge_path, user, volumes, command):
"""Run interactive shell for a challenge."""
config = lib.load_challenge_config(challenge_path)
try:
image_id = lib.build_challenge(challenge_path)
except RuntimeError as error:
raise click.ClickException(str(error)) from error
resolved_volumes = [path.resolve() for path in volumes]
logger.info("running %s as uid=%d, command=%s", challenge_path, user, list(command))
with lib.run_challenge(image_id, volumes=resolved_volumes) as (container, flag):
with lib.run_challenge(image_id, volumes=resolved_volumes, privileged=config["privileged"]) as (container, flag):
subprocess.run(["docker", "exec", f"--user={user}", "-it", container, *command])
3 changes: 2 additions & 1 deletion tools/pwnshop/src/pwnshop/commands/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def test_challenge(challenge_path):
challenge_path = pathlib.Path(challenge_path)
rendered = None
try:
config = lib.load_challenge_config(challenge_path)
rendered = lib.render_challenge(challenge_path)
image_id = lib.build_challenge(challenge_path)
tests = sorted(rendered.rglob("test*/test_*"))
Expand All @@ -75,7 +76,7 @@ def test_challenge(challenge_path):
for test in tests:
test_name = test.relative_to(rendered)
logger.debug("running test %s in %s", test_name, challenge_path)
with lib.run_challenge(image_id, volumes=[test]) as (container, _):
with lib.run_challenge(image_id, volumes=[test], privileged=config["privileged"]) as (container, _):
try:
run = subprocess.run(
["docker", "exec", "--user=1000:1000", container, f"{test}"],
Expand Down
25 changes: 21 additions & 4 deletions tools/pwnshop/src/pwnshop/lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,32 @@
import shutil
import subprocess
import tempfile
from typing import Iterable, Iterator, List, Optional, Sequence
from typing import Any, Dict, Iterable, Iterator, List, Optional, Sequence

import black
import jinja2
import pyastyle
import yaml

logger = logging.getLogger(__name__)

CHALLENGE_SEED = int(os.environ.get("CHALLENGE_SEED", "0"))

CHALLENGE_CONFIG_DEFAULTS: Dict[str, Any] = {
"privileged": False,
}


def load_challenge_config(challenge_path: pathlib.Path) -> Dict[str, Any]:
config = dict(CHALLENGE_CONFIG_DEFAULTS)
config_file = challenge_path / "challenge.yml"
if config_file.is_file():
logger.debug("loading challenge config from %s", config_file)
with open(config_file) as f:
user_config = yaml.safe_load(f) or {}
Comment on lines +31 to +33

Copilot AI Feb 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads challenge.yml with open(config_file) using the platform default encoding. To avoid locale-dependent failures, prefer config_file.open(encoding="utf-8") (or an explicit encoding consistent with the rest of the file loader logic).

Copilot uses AI. Check for mistakes.
config.update(user_config)
return config
Comment on lines +27 to +35

Copilot AI Feb 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yaml.safe_load() can return non-mapping values (e.g., a list/string) or a mapping with unexpected types. In those cases config.update(user_config) will raise at runtime, and privileged could end up non-bool (e.g., quoted "false" becomes a truthy string). Consider validating that user_config is a dict and that user_config.get("privileged") is either absent or a bool (otherwise raise a clear error or ignore with a warning).

Copilot uses AI. Check for mistakes.


class _NoSelfExtendLoader(jinja2.FileSystemLoader):
"""FileSystemLoader that prevents templates from extending/including themselves.
Expand Down Expand Up @@ -136,7 +152,7 @@ def ignore_git_crypt(current, names):

@contextlib.contextmanager
def run_challenge(
challenge_image: str, *, volumes: Optional[Sequence[pathlib.Path]] = None
challenge_image: str, *, volumes: Optional[Sequence[pathlib.Path]] = None, privileged: bool = False
) -> Iterator[tuple[str, str]]:
flag = "pwn.college{" + base64.b64encode(os.urandom(32)).decode() + "}"
env_options = []
Expand All @@ -149,6 +165,8 @@ def run_challenge(
logger.info("starting container for image %s", challenge_image)
if volumes:
logger.debug("mounting volumes: %s", volumes)
if privileged:
logger.debug("running container in privileged mode")
container = (
subprocess.check_output(
[
Expand All @@ -159,8 +177,7 @@ def run_challenge(
"--detach",
"--init",
"--user=0:0",
"--device=/dev/kvm",
"--cap-add=SYS_PTRACE",
*(["--privileged"] if privileged else ["--device=/dev/kvm", "--cap-add=SYS_PTRACE"]),
Comment on lines 154 to +180

Copilot AI Feb 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing challenge.yml to enable --privileged means an untrusted PR can cause CI to run fully privileged containers on the GitHub runner. That materially increases the blast radius vs the previous --device=/dev/kvm + --cap-add=SYS_PTRACE. Consider gating privileged mode behind an explicit CLI flag / env var that is disabled in CI by default (or an allowlist of known-safe challenge paths), and fail with a clear message when privileged is requested but not permitted.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ConnorNelson thoughts on this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we actually care about the integrity of the CI host, at least not currently.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure we adequately wipe the decryption keys? :-)

*env_options,
*[f"--volume={volume}:{volume}:ro" for volume in (volumes or [])],
challenge_image,
Expand Down
Loading