Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Crashing when files field is empty #166

Merged
merged 4 commits into from
Dec 30, 2024

Conversation

caffeine-addictt
Copy link
Contributor

@caffeine-addictt caffeine-addictt commented Dec 25, 2024

Looks like .get() does not set challenge_files to the default empty list when it is None, causing ctfcli to throw. Using the OR operator fixes this.

Reproduction steps

  1. Create any vaild challenge.yml
  2. Have an empty files field.
# challenge.yml

name: test
author: test
category: test
attribution: test
description: test
value: 10
type: standard
image: null
host: null
flags:
  - test
tags:
  - test
requirements:
  - Warmup
state: visible

# This is valid YML for an empty object
# but is loaded in as None type.
files:

Proof

image

@caffeine-addictt caffeine-addictt force-pushed the master branch 2 times, most recently from 4862a7d to 45f75d0 Compare December 25, 2024 12:42
Looks like .get() does not set challenge_files to the default empty list
when it is None, causing ctfcli to throw. A second potential error case
happens when directly using dict["files"] without checking for a None
value. Using the OR operator fixes this.

Signed-off-by: AlexNg <contact@ngjx.org>
This removes the repeated .get(). We can use the challenge_files from
the previous get() on line 764 as it remains unchanged.

Signed-off-by: AlexNg <contact@ngjx.org>
@ColdHeat
Copy link
Member

Can you make this look a little simpler?

Maybe something like

files = self.get("files") or []
for challenge_file in files:
    ...

But I would really prefer if we could have the .get method return a default if None. I don't really see why it doesn't.

@caffeine-addictt
Copy link
Contributor Author

caffeine-addictt commented Dec 26, 2024

@ColdHeat

Yea sure thing I can make it look simpler.

But I would really prefer if we could have the .get method return a default if None. I don't really see why it doesn't.

It was previously .get("files", []), but it still became a None value and threw. We could try overriding the dunder get just for files though...

~ nevermind, since its not a class but raw python dict's sometimes, this will be messy to do

Signed-off-by: AlexNg <contact@ngjx.org>
@ColdHeat
Copy link
Member

@caffeine-addictt Sorry to bother. Can you change all of the instances of the or strategy here so that you are doing the or in a seperate line.

@ColdHeat ColdHeat merged commit d78a030 into CTFd:master Dec 30, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants