-
Notifications
You must be signed in to change notification settings - Fork 40
Address challenge feedback from Discord #242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Original feedback: #242 (comment) Automated run notes: Feedback AddressedMoved the Removed the added Removed the confusing Restored all Validation
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 |
|---|---|---|
|
|
@@ -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. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Original feedback: #242 (comment) Automated run notes: Feedback AddressedMoved the Removed the added Removed the confusing Restored all Validation
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 |
|---|---|---|
| @@ -1 +1 @@ | ||
| {% include "../../../common/Dockerfile.j2" %} | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove all changes to web-security.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Original feedback: #242 (comment) Automated run notes: Feedback AddressedMoved the Removed the added Removed the confusing Restored all Validation
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" %} |
| 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" %} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| {% extends "../../common/Dockerfile.j2" %} | ||
| {% block additional_packages %}curl{% endblock %} |
| 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" %} |
| 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" %} |
| 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 %} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| {% include "../../../common/Dockerfile.j2" %} | ||
| {% include "../../common/Dockerfile.j2" %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did you put
leabefore 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.There was a problem hiding this comment.
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/leacomparison inchallenges/computing-101/hello-hackers/open-read-write-rip-relative/DESCRIPTION.mduntil afterleais introduced.Also updated
.agents/skills/authoring-challenges/SKILL.mdwith 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 fromchallenges/computing-101/the-stack-revisited/gdb-stack-align-2/DESCRIPTION.md.The file now matches
origin/mainfor this item.#242 (comment)
Removed the confusing
envp[]pointer-table sentence fromchallenges/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 toorigin/main, including deleting the PR-addedchallenges/web-security/common/Dockerfile.j2.The effective diff against
origin/mainnow 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.