Skip to content

Adding New Level: Lua Metatable hooks - #128

Open
TheDarkThief wants to merge 16 commits into
skills:mainfrom
TheDarkThief:main
Open

Adding New Level: Lua Metatable hooks#128
TheDarkThief wants to merge 16 commits into
skills:mainfrom
TheDarkThief:main

Conversation

@TheDarkThief

Copy link
Copy Markdown

Summary

Adding a new level demonstrating Lua metatables, and how a malicious actor can hook into them

Changes

Created a level where a user requests the client to populate a table, where a malicious actor gains remote code execution due to improper sanitation.

Task list

  • For workflow changes, I have verified the Actions workflows function as expected.
  • For content changes, I have reviewed the style guide.

@jkcso
jkcso self-requested a review March 9, 2025 21:27

@jkcso jkcso left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@TheDarkThief Thanks for your work! I left comments to improve it and accept it, good job man!

Comment thread Season-3/Level-1/code.lua Outdated
Comment thread Season-3/Level-1/code.lua Outdated


--- Generates a bitmap image
-- @param request: The table which we will populate with images

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I understand what you try to say but please rewrite it in a more explanatory way, such as for example that the table will contain pixels for the image that will be populated and it's being represented as a table.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Went into a bit more depth in the comment

Comment thread Season-3/Level-1/code.lua Outdated
Comment thread Season-3/Level-1/code.lua Outdated
Comment thread Season-3/Level-1/code.lua Outdated
Comment thread Season-3/Level-1/tests.lua Outdated
Comment thread Season-3/Level-1/tests.lua Outdated
Comment thread Season-3/Level-1/tests.lua Outdated
Comment thread Season-3/Level-1/tests.lua Outdated
Comment thread Season-3/Level-1/tests.lua Outdated
@TheDarkThief

Copy link
Copy Markdown
Author

I've made the requested changes, as well as switch to the Busted library making it easier to setup Github CI as well as make the error messages a bit clearer

@ZayanKhan-12

ZayanKhan-12 commented Aug 11, 2026

Copy link
Copy Markdown

Hi @TheDarkThief 👋 — just a heads-up on why this PR is currently stuck, in case it helps you revive it.

Since it was opened, main gained a full Season 4 (the ProdBot storyline), so this PR now conflicts with upstream:

  • add/add on Season-4/README.md
  • content conflict on .devcontainer/devcontainer.json
  • and Season-4/Level-1/ now overlaps the new Season 4 Level 1

To get it merge-ready you'd likely want to:

  • Re-home the level out of Season-4/ (e.g. into its own folder) so it no longer collides with the new Season 4, and
  • drop the .devcontainer / codeql-analysis.yml edits in this PR, which have gone stale — they'd remove the current ProdBot setup and downgrade actions/checkout@v7v4 and codeql-action@v4.37.3v3. Appending lua5.4 + luarocks + busted to the existing devcontainer is enough to run the tests.

The level itself is still solid — tests.lua and solution/solution_test.lua pass under busted, and hack.lua fails against the vulnerable code by design. @jkcso had said he wanted to accept this, so it'd be great to see it land.

(Edit: I'd briefly linked a rebased branch here but have since removed it — the notes above should be enough to update the PR directly.)

@TheDarkThief

Copy link
Copy Markdown
Author

Hello @ZayanKhan-12 , thank you for bringing this back up, I'll be able to work on it tomorrow.

@TheDarkThief

Copy link
Copy Markdown
Author

I have made the required changes, if something else needs work LMK and I'd be happy to get it done

@TheDarkThief

Copy link
Copy Markdown
Author

@jkcso Could you please review the changes

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.

5 participants