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

Adding S2_Level-3 : XSS Vulnerability #41 #42

Closed
wants to merge 8 commits into from

Conversation

viralvaghela
Copy link

Summary

Adding a new level-3 which demonstrates XSS Vulnerability in Flask API. #41

Changes

Describe the changes this pull request introduces.

  • Created a Python flask app that will return Planet information from the planet_data dictionary, endpoint /getPlanetInfo?planet= is vulnerable to XSS and will return malicious payload entered by the user

http://localhost:5000/getPlanetInfo?planet=<script>alert('1')</script>

image
image

Please do let me know if any changes/modifications are required!

@jkcso jkcso self-assigned this Jul 11, 2023
@jkcso jkcso added the new level Propose a new level in the game. Please make sure you follow the contribution guideline label Jul 11, 2023
Season2_Level-3/hint.md Outdated Show resolved Hide resolved
Congratulations on completing Level 2 !
Welcome to Level 3, where new challenges and mysteries await you. Here, you will face a heightened test of your skills and knowledge.

In this level, the website's vulnerability has become more elusive, requiring a deeper understanding of cross-site scripting (XSS) techniques. Your mission is to explore the intricate corners of the website's code, discover hidden flaws, and master the art of XSS exploitation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This must change, because we tell them what they are looking for (XSS).

We can just add a more generic story about space and find another way to make it a real-world use case, but without hinting or telling about the vulnerability name. We can discuss over video or I'm happy to take this task and show you what I suggest at a later stage. We can do this last.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great job! I really like the scenario and it brings a lot of value to our users!

I have the following thoughts to make it perfect:

  • If this level is going to be a level 3, it has to become a bit more difficult as now it's straight forward. Here we have 2 options: either do nothing and make it a level 1 or 2, or add some more code and make it a very interesting level 3 or even 4.
  • In case you want to make it a level 3 at least, then you have to add some defences in the code that actually implement input sanitisation, but the input sanitisation should have some gaps so that attackers can succeed. Right now, there's no defence at all, which makes it a straight forward. For inspiration, you can check out some common misconceptions or mistakes when developers implement input sanitisation, either through a bad regex or a bad library or manually. A super fun way to try to do it might be to strip off any input that has the word "alert" or in general any common XSS payloads so that players be like "hm, it's not XSS because they took care of it with their code defences"
  • This is optional but highly recommended if you want to experiment more, you can make it a level 4 by introducing another problem, like for example those we discussed before (insecure data storage, insecure host validation, etc).

Let me know what you think, we can go on video and discuss more 💯

Copy link
Author

Choose a reason for hiding this comment

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

Hi, thank you, I have done some modification and added a weak RE + Blocking script tag, it wont execute common XSS payloads
here is the code

@app.route('/', methods=['GET', 'POST'])
def index():
    if request.method == 'POST':
        planet = request.form.get('planet')
        sanitized_planet = re.sub(r'[<>(){}[\]]', '', planet)

        if 'script' in sanitized_planet.lower() :
            return '<h2>Blocked</h2></p>'

        elif sanitized_planet:
            details = get_planet_info(sanitized_planet)

            if planet:
                return f'<h2>Planet Details:</h2><p>{get_planet_info(planet)}</p>'
            else:
                return '<h2>Please enter a planet name.</h2>'

    return render_template('index.html')

But this kind of payload will be executed <<img src="x" onerror="alert(1)">>
image

details = get_planet_info(sanitized_planet)

if planet:
return f'<h2>Planet Details:</h2><p>{get_planet_info(planet)}</p>'

Check warning

Code scanning / CodeQL

Reflected server-side cross-site scripting Medium

Cross-site scripting vulnerability due to a
user-provided value
.
details = get_planet_info(sanitized_planet)

if planet:
return f'<h2>Planet Details:</h2><p>{get_planet_info(planet)}</p>'

Check warning

Code scanning / CodeQL

Reflected server-side cross-site scripting Medium

Cross-site scripting vulnerability due to a
user-provided value
.
@jkcso jkcso requested a review from abigailychen August 23, 2023 16:07
Co-authored-by: Courtney Wilson <77312589+cmwilson21@users.noreply.github.com>
Copy link

@RasmusWL RasmusWL left a comment

Choose a reason for hiding this comment

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

A few minor comments from me 😊

Season2_Level-3/solution.md Outdated Show resolved Hide resolved
return render_template('index.html')

@app.route('/getPlanetInfo', methods=['GET'])
def get_planet_info_endpoint():

Choose a reason for hiding this comment

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

Maybe I'm overlooking something, but from my point of view it doesn't look like the get_planet_info_endpoint is offering any additional value besides the index request handler? (that is, it's code is almost identical, and is vulnerable to the same attack). From my point of view, we might be able to make the code simpler by simply removing this part of the code?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@viralvaghela what do you think?

Co-authored-by: Rasmus Wriedt Larsen <rasmuswriedtlarsen@gmail.com>
@jkcso
Copy link
Collaborator

jkcso commented Sep 20, 2023

As the second season levels are hosted in an internal repo for staff testing and content review, I am now closing this public PR. Thanks again for your valuable contribution and it will be soon merge into the public repo :-)

@jkcso jkcso closed this Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new level Propose a new level in the game. Please make sure you follow the contribution guideline
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants