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

User Profile with SSTI and SSRF Bugs #655

Merged
merged 36 commits into from Nov 3, 2018

Conversation

CaptainFreak
Copy link
Contributor

PR submitted for suggestion, improvement and review.

/profile is a new server side rendered page with jade template engine. It has functionality of Submitting and changing Username, Email and Profile Picture (From URL/Computer). Username field contains standard SSTI Bug and URL field contains SSRF Bug. The scenario for these challenges is yet to be decided.

@ghost ghost assigned CaptainFreak Jul 25, 2018
@ghost ghost added the in progress label Jul 25, 2018
@CaptainFreak
Copy link
Contributor Author

So the one liner payload of SSTI - RCE is #{root.process.mainModule.require('child_process').exec('whoami')}.

@J12934
Copy link
Member

J12934 commented Jul 25, 2018

👍 Nice!

I've tried to come up with a good scenario for a SSRF... But nothing really comes to my mind. We are simply missing the large "internal juice-shop company" network 😉
A simpler solution would be to turn this image upload into a path traversal challenge. E.g. user sets ../../../../etc/passwd as their image. If we come up with a good scenario we could then "patch" the path traversal 😅 and replace it with SSRF.

The SSTI => RCE is a bit easier. The problems with RCE is that if you have the RCE you basically own the server. So we really can' ask the user to do anything he/she could not do via the other RCE vulns. (Please tell me if this assumption is wrong)
So we just need to come up with a interesting scenario, a real attacker might also do.

....

Ok while writing this i had a (hopefully) interesting idea which would work for both challenges.
(This might be crazy, please tell if it is)

SSTI Challenge: Download and execute this malware [link to malware] on the server or decompile the malware and find a way to reproduce what it does.

This provides a link to a compiled malware executable which simply makes a request to the juice-shop (new hidden endpoint, http://localhost:3000/internal/solve/challenge-ssti?key=some_long_key_so_it_cant_be_guessed).

So the malware sends the request to the endpoint the endpoint checks where the request is coming from and only response from local requests. So there are two ways to solve this:

  1. Do the SSTI => RCE and let the "malware" run on the server. This triggers the endpoint and solves the challenge.
  2. Download the "malware" and figure out that it just sends a request to this endpoint. When the user tries to access the url directly they will get a message that this only triggers for internal requests. So the have to do the SSRF so that the server does the request itself. This would also solve the challenge.

This is still a pretty raw idea. And there a a couple of problems, which come to my mind.

  1. On local juice-shop install it would be hard / impossible to detect whether the request was done by the user or come via the server / "malware" as the ip addrs are identical. In docker this would should work...
  2. Malware reverse engineering was never really part of the juice-shop. But this might be a nice addition.

Sorry for the long text i hope it was kind of clear what i meant with all of that 😅

@J12934
Copy link
Member

J12934 commented Jul 25, 2018

Okay other than the craziness, also some generall things about this:

  1. The navbar is at the moment a "mirror" of the real single page app navbar. It would suggest replacing all the links and buttons in there with just a "<- back" button. This makes it easier as we dont have do maintain both navbars. Also makes it even more obvious that this page is different than the others.
  2. The profile page is a bit useless at the moment as you can only see your own profile. Its ore like a personal information page. We should consider to to include the profile pictures in the product comments and use the user name instead of the email the product reviews.

@CaptainFreak
Copy link
Contributor Author

CaptainFreak commented Jul 26, 2018

Very Very nice idea tbh @J12934 . I really like the malware idea. Only problem you mentioned is checking from where request came right? We can solve that with the User-Agent header .. we will just allow the header of npm package request and the user-agent our malware will have.

@CaptainFreak
Copy link
Contributor Author

So SSRF challenge only gets solved this way @J12934 ?

@CaptainFreak
Copy link
Contributor Author

I am so excited to implement this .. I will need guidance on how to design that malware binary so that its platform independent.

@georgeprichici
Copy link

I love the malware idea, since this is something I was thinking to implement as well, since I need it for a demo.

I was looking at the profile page, but I've also considered adding images to product details comments. The goal was to find a way to do either RCE or code injection from images EXIF tags.

The problem is that for user profile EXIF is not really relevant, while for images added to comments we might be interested to display timestamp, geolocation or similar. The question was more towards where that info should be read: server or client side. And this might solve the cross platform malicious file.

The only problem is that it should be a nice story around it:

  • if you'll see the extra details for the image on the client side, its clear that EXIF is being parsed and maybe it can be leveraged
  • if there's nothing on the client side, it would be very difficult to guess the way to solve the challenge.

@J12934
Copy link
Member

J12934 commented Jul 26, 2018

@georgeprichici that sounds exiting!
Could you maybe open a new Issue so we can track this possible challenge?

@J12934
Copy link
Member

J12934 commented Jul 26, 2018

@CaptainFreak UserAgent would at least be another hurel so that is isnt to easy on a local setup. But the user could obviously just set the UserAgent in the request him/herself...

@CaptainFreak
Copy link
Contributor Author

In that case he will have to guess that ? Atleast for request package's user agent. We can send something unique with request to identify....

c-0.605,1.881-1.478,3.674-2.632,5.304c-0.291,0.411-0.563,0.759-0.801,1.03V38.8c0,1.223,0.691,2.342,1.785,2.888l8.467,4.233
c0.508,0.254,0.967,0.575,1.39,0.932c5.71-4.762,9.399-11.882,9.536-19.9C53.246,12.32,41.587,0.254,26.953,0.004z"/>
</g>
<g>
Copy link
Member

Choose a reason for hiding this comment

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

These gs look like a bit too much 😉

entry.autodrain()
}
})
if (file === undefined && req.body.imageUrl !== undefined) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should move this new functionality into a new endpoint, instead of reusing the existing file-upload.
The file upload handler is already pretty messy.

Copy link
Member

Choose a reason for hiding this comment

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

I would actually create two new endpoints.

  1. for the "Upload by URL". Something like: POST /profile/image/url
  2. for the "Upload by File". Something like: POST /profile/image/file

@bkimminich bkimminich closed this Sep 30, 2018
@ghost ghost removed the in progress label Sep 30, 2018
@bkimminich
Copy link
Member

bkimminich commented Sep 30, 2018

I merged gsoc-challenges and gsoc-frontend one last time into gsoc-integration and will rename that one into release-v8 soon. Please use that one as new base branch and resubmit PR.

@bkimminich bkimminich changed the base branch from gsoc-challenges to release-v8 October 2, 2018 14:14
@bkimminich bkimminich reopened this Oct 2, 2018
@ghost ghost added the in progress label Oct 2, 2018
@bkimminich
Copy link
Member

@CaptainFreak @J12934 PR #655 is now reopened onto release-v8 branch with all merge conflicts resolved (I think...) - Feel free to wrap it up with the mentioned API tests and any idea forat least "sandboxing against the silliest of accidents"... 😁

@bkimminich
Copy link
Member

Alternatively, you can also move both challenges into xchallenges.yml to disable them for now. That way the profile feature could be part of the initial v8.0 release and the challenges just come with v8.1 at some later point in time.

@J12934
Copy link
Member

J12934 commented Oct 2, 2018

Okay will try to implement the tests tomorrow 😉
Do you have a concrete time frame when you want to do the 8.0 release?

@bkimminich
Copy link
Member

Before German OWASP Day, so 1st November is my latest "merge-to-master"-goal.

bkimminich and others added 4 commits October 2, 2018 18:43
As the profile page doesnt exists outside the angular router, `routerLink` cannot be used to link to it.
@bkimminich bkimminich changed the base branch from release-v8 to develop October 9, 2018 10:40
@CaptainFreak
Copy link
Contributor Author

Only API tests are remaining right @J12934 ? @bkimminich do you need anything else to be added for merge ?

@bkimminich
Copy link
Member

bkimminich commented Oct 10, 2018

any idea forat least "sandboxing against the silliest of accidents"... 😁

That is still missing, right? That seems more important to me than the API tests. Currently most API tests are failing with 500 errors anyway (see #640).

@bkimminich bkimminich removed this from the GSoC - Challenge Pack 2018 milestone Oct 15, 2018
@CaptainFreak
Copy link
Contributor Author

Hey @bkimminich @J12934 Can you give me a to-do on this ? for sandboxing.

@J12934
Copy link
Member

J12934 commented Oct 27, 2018

Hi @CaptainFreak
Yeah the sandboxing is a pretty hard thing to do for the way the SSTI Challenge is implemented, because we actually want to let the user do a fully fletched remote code execution.

I'd say there are to ways to do it:

  1. Find a way to prevent actual harmful scripts from being run. This is probably quite hard to implement. The best way i could think of would be to have a whitelist of non-harmfull commands relevant for solving this challenge. This would not have to be perfect, but should be able to prevent obvious mistakes.
  2. Make this challenge "OptIn", meaning the challenge is disabled and the user would have to set a config setting like potentially-harmful-challenges: true to enable the challenge. This would be easier but would probably also mean that ~95% of users would never see the challenge. (The percentage is made up but should be realistic 😉)

@bkimminich
Copy link
Member

There now is a challenges.safetyOverride property in the config that is false by default. When set to true all disabled challenges are enabled independent of environment restrictions. At the moment it is only needee for the XXE challenges. We could disabled the dangerous RCE on all envs unless safety is turned off.

@bkimminich bkimminich merged commit 3847c01 into juice-shop:develop Nov 3, 2018
@ghost ghost removed the in progress label Nov 3, 2018
@lock
Copy link

lock bot commented Nov 4, 2019

This thread has been automatically locked because it has not had recent activity after it was closed. 🔒 Please open a new issue for regressions or related bugs.

@lock lock bot locked and limited conversation to collaborators Nov 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants