Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ path:
**THIS IS STILL NOT WHAT WE WANT!**
Why? Because it _reads_ the 8 bytes at `[rip+path]` into `rdi` rather than put the address of those bytes into `rdi`.
`rdi` would end up holding the values `'f'`, and `'l'`, and so on, but the `open` syscall needs the address and not the values.
Think of `path` as the address where the first byte of `"/flag\0"` lives: `mov` copies bytes from there, while `lea` copies the address so the kernel can walk those bytes until the null byte.

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.

How did you put lea before the place where we freaking introduce it? OBVIOUSLY this sentence should be later. Like, it's insane that I have to type this; edit whatever relevant skill to fix it so you don't make this mistake again.

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.

THIS MESSAGE IS GENERATED BY AN AUTOMATED PROCESS.

Addressed this feedback and pushed commit e82c47c0.

Original feedback: #242 (comment)

Automated run notes:

Feedback Addressed

#242 (comment)

Moved the mov/lea comparison in challenges/computing-101/hello-hackers/open-read-write-rip-relative/DESCRIPTION.md until after lea is introduced.
Also updated .agents/skills/authoring-challenges/SKILL.md with a local-ordering rule so future DESCRIPTION edits do not use an instruction, term, or comparison before introducing it.

#242 (comment)

Removed the added envp[] pointer-table wording from challenges/computing-101/the-stack-revisited/gdb-stack-align-2/DESCRIPTION.md.
The file now matches origin/main for this item.

#242 (comment)

Removed the confusing envp[] pointer-table sentence from challenges/computing-101/the-stack-revisited/mem-stack-align/DESCRIPTION.md.
Kept the direct clarification that the environment string includes FOO=, the value, and the trailing null byte.

#242 (comment)

Restored all challenges/web-security/** files to origin/main, including deleting the PR-added challenges/web-security/common/Dockerfile.j2.
The effective diff against origin/main now contains no web-security changes.

Validation

git diff --check: clean.

nix develop --command pwnshop test challenges/computing-101/hello-hackers/open-read-write-rip-relative: passed, 1 challenge / 1 testcase.

No web-security tests were run because the requested resolution was to remove all web-security changes from the PR.


Luckily, there is an instruction that is _almost_ a read, but instead _does_ put the address that would have been read into `rdi` (or whatever other register).
That instruction is **l**oad **e**ffective **a**ddress (the word _effective_ here refers to the CPU figuring out all the calculations it needs to do, such as adding an offset to the instruction pointer in this case):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,54 @@ def split_operands(insn):
def normalize_reg(reg):
return checker.SUBREG_TO_64.get(reg, reg)

def instruction_sets_reg(insn, reg, value):
if not insn.op_str:
return False

if insn.mnemonic == "mov":
dst, src = split_operands(insn)
return normalize_reg(dst) == reg and normalize_reg(src) == value

if value == "0" and insn.mnemonic == "xor":
dst, src = split_operands(insn)
return normalize_reg(dst) == reg and normalize_reg(src) == reg

return False

def instruction_writes_reg(insn, reg):
if insn.mnemonic not in {"lea", "mov"} or not insn.op_str:
if insn.mnemonic not in {"lea", "mov", "xor"} or not insn.op_str:
return False
return normalize_reg(split_operands(insn)[0]) == reg

def rax_value_before(disas, idx):
for insn in reversed(disas[:idx]):
if instruction_sets_reg(insn, "rax", "0"):
return "0"
if insn.mnemonic == "mov" and insn.op_str:
dst, src = split_operands(insn)
if normalize_reg(dst) == "rax":
return normalize_reg(src)
return None

def assert_write_count_from_read(disas):
syscalls = [i for i, insn in enumerate(disas) if insn.mnemonic == "syscall"]
read_i = next((i for i in syscalls if rax_value_before(disas, i) == "0"), None)
write_i = next((i for i in syscalls if rax_value_before(disas, i) == "1"), None)
assert read_i is not None, "You need to invoke the read syscall (set rax to 0)!"
assert write_i is not None, "You need to invoke the write syscall (set rax to 1)!"
assert read_i < write_i, "You need to read the data before you write it back out!"

rdx_after_read = [
insn
for insn in disas[read_i + 1:write_i]
if instruction_sets_reg(insn, "rdx", "rax")
]
assert rdx_after_read, (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Require the final rdx write to come from read

When a solution does mov rdx, rax after the read and then overwrites rdx before the write syscall, this new check still passes because it only requires that the read-return copy appears somewhere between the two syscalls. check_runtime only asserts the program exits with code 42, so a program can pass while writing zero bytes or a hardcoded length instead of printing exactly the flag; keep tracking rdx writes and require the last one before write to come from rax, as the shared helper did.

Useful? React with 👍 / 👎.

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.

THIS MESSAGE IS GENERATED BY AN AUTOMATED PROCESS.

Addressed this feedback and pushed commit c24b9c84.

Original feedback: #242 (comment)

Automated run notes:

#242 (comment)

Changed challenges/computing-101/hello-hackers/open-read-write-rip-relative/challenge/.py/chal.py so the challenge-local assert_write_count_from_read tracks all recognized rdx writes between the read and write syscalls and requires the last one before write to set rdx from rax.
This closes the bypass where mov rdx, rax appeared after read but rdx was overwritten before write.

#242 (review)

No separate repo change was needed for the parent review entry; the only actionable instruction in that review was the inline comment above.

Validation:

  • Ran nix develop --command pwnshop test challenges/computing-101/hello-hackers/open-read-write-rip-relative; it passed with 1 challenge and 1 testcase.
  • Ran a focused synthetic-disassembly check against the helper confirming a valid read-to-write rdx flow still passes and the stale-rdx sequence now raises AssertionError.

"write's length (rdx) must come from read's return value (rax).\n"
"read returns how many bytes it actually read, so after your read syscall do\n"
"`mov rdx, rax` --- write exactly that many bytes --- rather than hardcoding a length."
)

def check_disassembly(disas):
mov_operands = checker.mov_operands(disas)
syscall_indices = [i for i, insn in enumerate(disas) if insn.mnemonic == "syscall"]
Expand Down Expand Up @@ -65,7 +108,7 @@ def check_disassembly(disas):
"You need to set rax to 2, the syscall number for open!"
)

assert ['rax', '0'] in mov_operands, (
assert any(instruction_sets_reg(insn, "rax", "0") for insn in disas), (
"You need to set rax to 0, the syscall number for read!"
)

Expand All @@ -82,7 +125,7 @@ def check_disassembly(disas):

# Write back exactly what you read: write's length (rdx) must come from read's
# return value (rax), the idiom you learned in read-exact --- not a hardcoded count.
checker.assert_write_count_from_read(disas)
assert_write_count_from_read(disas)

exit_syscall = syscall_indices[-1]
rax_writes_before_exit = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,5 @@ hacker@dojo:~$ /challenge/program
```

This allows you to have very finegrained control over your environment.
When you build that one variable, count the bytes in its environment string (`NAME=`, the value, and its trailing null byte), not a separate `envp[]` pointer-table slot.

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.

This seems to force confusion into the description. What are you trying to achieve here? If it's relevant, don't do it in a way that confuses the learner further.

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.

THIS MESSAGE IS GENERATED BY AN AUTOMATED PROCESS.

Addressed this feedback and pushed commit e82c47c0.

Original feedback: #242 (comment)

Automated run notes:

Feedback Addressed

#242 (comment)

Moved the mov/lea comparison in challenges/computing-101/hello-hackers/open-read-write-rip-relative/DESCRIPTION.md until after lea is introduced.
Also updated .agents/skills/authoring-challenges/SKILL.md with a local-ordering rule so future DESCRIPTION edits do not use an instruction, term, or comparison before introducing it.

#242 (comment)

Removed the added envp[] pointer-table wording from challenges/computing-101/the-stack-revisited/gdb-stack-align-2/DESCRIPTION.md.
The file now matches origin/main for this item.

#242 (comment)

Removed the confusing envp[] pointer-table sentence from challenges/computing-101/the-stack-revisited/mem-stack-align/DESCRIPTION.md.
Kept the direct clarification that the environment string includes FOO=, the value, and the trailing null byte.

#242 (comment)

Restored all challenges/web-security/** files to origin/main, including deleting the PR-added challenges/web-security/common/Dockerfile.j2.
The effective diff against origin/main now contains no web-security changes.

Validation

git diff --check: clean.

nix develop --command pwnshop test challenges/computing-101/hello-hackers/open-read-write-rip-relative: passed, 1 challenge / 1 testcase.

No web-security tests were run because the requested resolution was to remove all web-security changes from the PR.

In this challenge, you'll use this finegrained control to line up addresses in a slightly more realistic setting, but keep the capability in mind for other situations!
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,6 @@ hacker@dojo:~$ env -i /challenge/program
hacker@dojo:~$ env -i FOO=xxxxxxxx /challenge/program
```

Remember that the whole environment string is placed on the stack, so `FOO=` and the trailing null byte count toward the shift too.
Remember that the whole environment string is placed on the stack, so `FOO=`, the value, and the trailing null byte count toward the shift.
Count those string bytes, not a separate `envp[]` pointer-table slot.

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.

Again, where did envp pointer-table slot come in? This just confuses the learner! If there is a point to this, make it in a way that's not confusing.

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.

THIS MESSAGE IS GENERATED BY AN AUTOMATED PROCESS.

Addressed this feedback and pushed commit e82c47c0.

Original feedback: #242 (comment)

Automated run notes:

Feedback Addressed

#242 (comment)

Moved the mov/lea comparison in challenges/computing-101/hello-hackers/open-read-write-rip-relative/DESCRIPTION.md until after lea is introduced.
Also updated .agents/skills/authoring-challenges/SKILL.md with a local-ordering rule so future DESCRIPTION edits do not use an instruction, term, or comparison before introducing it.

#242 (comment)

Removed the added envp[] pointer-table wording from challenges/computing-101/the-stack-revisited/gdb-stack-align-2/DESCRIPTION.md.
The file now matches origin/main for this item.

#242 (comment)

Removed the confusing envp[] pointer-table sentence from challenges/computing-101/the-stack-revisited/mem-stack-align/DESCRIPTION.md.
Kept the direct clarification that the environment string includes FOO=, the value, and the trailing null byte.

#242 (comment)

Restored all challenges/web-security/** files to origin/main, including deleting the PR-added challenges/web-security/common/Dockerfile.j2.
The effective diff against origin/main now contains no web-security changes.

Validation

git diff --check: clean.

nix develop --command pwnshop test challenges/computing-101/hello-hackers/open-read-write-rip-relative: passed, 1 challenge / 1 testcase.

No web-security tests were run because the requested resolution was to remove all web-security changes from the PR.

You're not modifying the program at all, just changing how it's launched, which influences where its data ends up!
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Above, the following sequence of events took place:
Now it's your turn!
In this challenge, we have `/challenge/hack`, `/challenge/the`, and `/challenge/planet`.
Run the `/challenge/hack` command, and duplicate its output as input to both the `/challenge/the` and the `/challenge/planet` commands!
Remember that `tee` can take multiple output targets, and `>(command)` is one kind of target.
Scroll back through the previous challenges "Duplicating piped data with tee" and "Process substitution for input" if you need a refresher on this method.

----
Expand Down
4 changes: 4 additions & 0 deletions challenges/web-security/DESCRIPTION.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
You have learned Linux and HTTP.
Now, let's put these together!

When a level asks you to run `/challenge/server`, the GUI browser is optional.
Start the server in one terminal and make requests from another terminal to `http://challenge.localhost/`; the challenge images include `curl` for terminal-only work.
Browser-specific levels will tell you when they need the GUI or `/challenge/victim`.

Web content is served up via the internet by _web servers_, and like everything else, these web servers, and the pages that they serve up, contain vulnerabilities!
In this module, you will wrap yourself in the mysteries of the web, exploring various types of vulnerabilities that can occur.
As you work through this module, keep in mind, these aren't theoretical curiosities: these are common, critical vulnerabilities that occur _all the time_ in the modern web and can lead to massive data breaches, account takeover, and more.
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}

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.

Remove all changes to web-security.

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.

THIS MESSAGE IS GENERATED BY AN AUTOMATED PROCESS.

Addressed this feedback and pushed commit e82c47c0.

Original feedback: #242 (comment)

Automated run notes:

Feedback Addressed

#242 (comment)

Moved the mov/lea comparison in challenges/computing-101/hello-hackers/open-read-write-rip-relative/DESCRIPTION.md until after lea is introduced.
Also updated .agents/skills/authoring-challenges/SKILL.md with a local-ordering rule so future DESCRIPTION edits do not use an instruction, term, or comparison before introducing it.

#242 (comment)

Removed the added envp[] pointer-table wording from challenges/computing-101/the-stack-revisited/gdb-stack-align-2/DESCRIPTION.md.
The file now matches origin/main for this item.

#242 (comment)

Removed the confusing envp[] pointer-table sentence from challenges/computing-101/the-stack-revisited/mem-stack-align/DESCRIPTION.md.
Kept the direct clarification that the environment string includes FOO=, the value, and the trailing null byte.

#242 (comment)

Restored all challenges/web-security/** files to origin/main, including deleting the PR-added challenges/web-security/common/Dockerfile.j2.
The effective diff against origin/main now contains no web-security changes.

Validation

git diff --check: clean.

nix develop --command pwnshop test challenges/computing-101/hello-hackers/open-read-write-rip-relative: passed, 1 challenge / 1 testcase.

No web-security tests were run because the requested resolution was to remove all web-security changes from the PR.

{% include "../../common/Dockerfile.j2" %}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
2 changes: 1 addition & 1 deletion challenges/web-security/cmdi-env/challenge/Dockerfile.j2
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
2 changes: 2 additions & 0 deletions challenges/web-security/common/Dockerfile.j2
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{% extends "../../common/Dockerfile.j2" %}
{% block additional_packages %}curl{% endblock %}
2 changes: 1 addition & 1 deletion challenges/web-security/path-traversal-1/DESCRIPTION.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ We've implemented a simple web server for you --- it will serve up files from /c
Can you trick it into giving you the flag?

The webserver program is `/challenge/server`.
You can run it just like any other challenge, then talk to it over HTTP (using a different terminal or a web browser).
You can run it just like any other challenge, then talk to it over HTTP from another terminal with `curl` or from a web browser.
We recommend reading through its code to understand what it is doing and to find the weakness!

----
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
2 changes: 1 addition & 1 deletion challenges/web-security/sqli-blind/challenge/Dockerfile.j2
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
2 changes: 1 addition & 1 deletion challenges/web-security/sqli-pin/challenge/Dockerfile.j2
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
2 changes: 1 addition & 1 deletion challenges/web-security/sqli-union/challenge/Dockerfile.j2
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/dockerfile.selenium.j2" %}
8 changes: 7 additions & 1 deletion challenges/web-security/xss-stored-alert/challenge/victim.j2
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
{%- include "../../common/victim-requests.py.j2" -%}
{%- extends "../../../common/victim-selenium.py.j2" -%}

{% block setup %}
{{- super() -}}
{%- set challenge.specify_url = False -%}
{%- set challenge.reward_alert = True -%}
{% endblock %}
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{% include "../../../common/Dockerfile.j2" %}
{% include "../../common/Dockerfile.j2" %}
Loading