Skip to content
This repository was archived by the owner on Dec 19, 2023. It is now read-only.

Avoid CSRF #2

Merged
JamieSlome merged 2 commits into from
Jan 14, 2021
Merged

Avoid CSRF #2

JamieSlome merged 2 commits into from
Jan 14, 2021

Conversation

@alromh87
Copy link

@alromh87 alromh87 commented Dec 8, 2020 •
edited by JamieSlome
Loading

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-other-node-login

⚙️ Description *

node-login is a template for quickly building login systems on top of Node.js & MongoDB. It is vulnerable to CSRF attacks in Update and Delete profile actions

💻 Technical Description *

Avoid CSRF by using csrf token using csurf

🐛 Proof of Concept (PoC) *

  1. Download and setup node-login
  2. Go to http://localhost:3000/signup and create an account
  3. Create payload and serve trough web
<html>
  <!-- CSRF PoC - generated by Burp Suite Professional -->
  <body>
  <script>history.pushState('', '', '/')</script>
    <form action="http://localhost:3000/home" method="POST">
      <input type="hidden" name="name" value="hacked" />
      <input type="hidden" name="email" value="hacked&#64;test&#46;com" />
      <input type="hidden" name="country" value="Afghanistan" />
      <input type="hidden" name="pass" value="hacked" />
      <input type="submit" value="Submit request" />
    </form>
  </body>
</html>
  1. Open payload from same browser as logged in user and click on Submit button
  2. User data has been tempered with
    Captura de pantalla de 2020-12-07 21-08-45

🔥 Proof of Fix (PoF) *

After fix data is unafected as csrf token is requiered for post actions

Captura de pantalla de 2020-12-17 10-02-44

👍 User Acceptance Testing (UAT)

All functinality is unafected
Captura de pantalla de 2020-12-07 21-44-31

@JamieSlome JamieSlome requested a review from mzfr December 16, 2020 14:21
mzfr
mzfr reviewed Dec 17, 2020
View reviewed changes
Copy link

@mzfr mzfr left a comment

Choose a reason for hiding this comment

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

Hey, The fix looks good but I just think that instead of showing the complete debug message there should be some small error that won't give away internal paths etc.

@alromh87 alromh87 requested a review from mzfr December 17, 2020 16:02
@alromh87
Copy link
Author

alromh87 commented Dec 17, 2020

@mzfr thanks for the comment, updated

mzfr
mzfr approved these changes Dec 17, 2020
View reviewed changes
Copy link

@mzfr mzfr left a comment

Choose a reason for hiding this comment

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

Looks good now

Mik317
Mik317 suggested changes Dec 17, 2020
View reviewed changes
Copy link

@Mik317 Mik317 left a comment

Choose a reason for hiding this comment

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

Hi @alromh87 😄,
thanks for the great fix 👍

Could you add the protection also for the Delete action?

Cheers,
Mik

@alromh87
Copy link
Author

alromh87 commented Jan 14, 2021 •
edited
Loading

Hello @Mik317 I think delete action is already taken care of, unless I'm missing another endpoint, you can try with this POC:

<html>
  <body>
  <script>history.pushState('', '', '/')</script>
    <form action="http://localhost:3000/delete" method="POST">
      <input type="submit" value="Submit request" />
    </form>
  </body>
</html>

And you should get:
Captura de pantalla de 2021-01-13 22-26-24

Before the fix you would get:
Captura de pantalla de 2021-01-13 22-30-58

Let me know what you think

@alromh87 alromh87 requested a review from Mik317 January 14, 2021 04:27
@JamieSlome JamieSlome merged commit 55ea3b2 into 418sec:master Jan 14, 2021
@huntr-helper huntr-helper mentioned this pull request Jan 14, 2021
Open
@huntr-helper
Copy link

huntr-helper commented Jan 14, 2021

Congratulations alromh87 - your fix has been selected! 🎉

Thanks for being part of the community & helping secure the world's open source code.
If you have any questions, please respond in the comments section, or hit us up on Discord. Your bounty is on its way - keep hunting!

Come join us on Discord

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@bbeale bbeale Awaiting requested review from bbeale

@mufeedvh mufeedvh Awaiting requested review from mufeedvh

@Mik317 Mik317 Awaiting requested review from Mik317

1 more reviewer

@mzfr mzfr mzfr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@alromh87 @huntr-helper @mzfr @Mik317 @JamieSlome