Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Bonus Payload" XSS payload broken from HTML entity encoding [🐛] #104

Closed
bkimminich opened this issue May 6, 2021 · 7 comments
Closed

Comments

@bkimminich
Copy link
Member

🐛 Bug report

Description

At least in CTFd the payload as part of the "Bonus Payload" challenge description is this

<iframe width="100%" height="166" scrolling="no" frameborder="no" allow="autoplay" src="https://w.soundcloud.com/player/?url=https%3A//api.soundcloud.com/tracks/771984076&amp;color=%23ff5500&amp;auto_play=true&amp;hide_related=false&amp;show_comments=true&amp;show_user=true&amp;show_reposts=false&amp;show_teaser=true&quot;&gt;&lt;/iframe>

where on the Juice Shop score board it is actually

<iframe width="100%" height="166" scrolling="no" frameborder="no" allow="autoplay" src="https://w.soundcloud.com/player/?url=https%3A//api.soundcloud.com/tracks/771984076&color=%23ff5500&auto_play=true&hide_related=false&show_comments=true&show_user=true&show_reposts=false&show_teaser=true"></iframe>

Copy/paste of the payload from CTFd doesn't work, which might be confusing/frustrating to users.

@bkimminich
Copy link
Member Author

Issue can still be reproduced with CTFd 3.4 and Juice Shop CTF Extension 9.0.0:

image

@meelunae
Copy link
Contributor

meelunae commented Mar 15, 2024

Mirroring comment posted on Slack:
Iit seems like either CTFd (more likely) or the browser itself are automatically incapsulating the Soundcloud link into an < a href > tag due to an unescaped "https://" at the beginning of the url, which led into having an empty src="" tag and a href which included all the other tags which did not get parsed correctly. This is what the import looks like on my end when replacing https:// with https%3A// as done in the second part of the URL, which is what I believe to be expected behavior; if this is the solution that we'd expect, I believe this change should happen in the main juice shop repo as the Challenges API is returning the unescaped https:// URL.
image

@Zeeshan12340
Copy link
Contributor

Hello, I've tested this bug locally, the issue is indeed that the first https:// is not URL encoded so I manually changed it and it appears to be working. The added code is below, should I create a PR with this or is this fix too specific. The alternative is to change the challenge description in juice shop so that /api/Challenges returns the correct description but that can break other stuff as mentioned in the previous closed PR.

diff --git a/index.js b/index.js
index 3763f4e..1baac1c 100644
--- a/index.js
+++ b/index.js
@@ -104,6 +104,12 @@ const juiceShopCtfCli = async () => {
       fetchCodeSnippets(answers.juiceShopUrl, argv.ignoreSslWarnings, answers.insertHintSnippets === options.noHintSnippets)
     ])

+    for (const challenge of challenges) {
+      if (challenge.name === 'Bonus Payload') {
+        challenge.description = challenge.description.replace('https://', 'https%3A//');
+      }
+    }
+
     await generateCtfExport(answers.ctfFramework || options.ctfdFramework, challenges, {
       juiceShopUrl: answers.juiceShopUrl,
       insertHints: answers.insertHints,

image

@bkimminich
Copy link
Member Author

This seems fine, it's a workaround but it seems that's what it takes in this very specific case... 😆

@meelunae
Copy link
Contributor

My issue with this approach (and the reason I closed the PR) is that the iframe payload gets broken because the %3A encoding does not get resolved correctly when you use it as beginning of url in the src tag :(

@Zeeshan12340
Copy link
Contributor

Hmm, yeah, the payload does not work in the challenge if https%3A// is used but otherwise, it is displayed correctly.

@Zeeshan12340
Copy link
Contributor

Okay, it appears that using html encoding instead of URL encoding fixes it completely.

diff --git a/index.js b/index.js
index 1baac1c..d409bc3 100644
--- a/index.js
+++ b/index.js
@@ -106,7 +106,7 @@ const juiceShopCtfCli = async () => {

     for (const challenge of challenges) {
       if (challenge.name === 'Bonus Payload') {
-        challenge.description = challenge.description.replace('https://', 'https%3A//');
+        challenge.description = challenge.description.replace('https://', 'https&colon;//');
       }
     }

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants