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

Cross-site scripting vulnerability in ParseDown #530

Closed
prodigysml opened this issue Sep 22, 2017 · 9 comments · Fixed by #495
Closed

Cross-site scripting vulnerability in ParseDown #530

prodigysml opened this issue Sep 22, 2017 · 9 comments · Fixed by #495

Comments

@prodigysml
Copy link

<?php

include("parsedown/Parsedown.php");

echo Parsedown::instance()->text('Hello ![google" onerror="alert(1)](http://google.com)'); 

The code above simply creates an instance of your Parsedown parser and then creates an image in markdown. This creation of the image is vulnerable to XSS. I would advice something like HTML encoding some of the input in certain places (especially for things like quotes).

@cebe
Copy link
Contributor

cebe commented Sep 22, 2017

duplicate of #161

@aidantwoods
Copy link
Collaborator

Fixed in #495 if you’d like to test it.

@embluk
Copy link

embluk commented Sep 29, 2017

@aidantwoods Can I ask why this has not been merged into master yet, as this XSS vulnerability is quite serious?

@aidantwoods
Copy link
Collaborator

Can I ask why this has not been merged into master yet, as this XSS vulnerability is quite serious?

@embluk I'd love to see it merged into master, but unfortunately I don't have write rights on this repo.

If you're a composer user you can take a look at https://packagist.org/packages/aidantwoods/secureparsedown, or https://github.com/aidantwoods/SecureParsedown otherwise to apply the patch via a class extension.

@embluk
Copy link

embluk commented Sep 29, 2017

@aidantwoods Thanks for all your great work on this!

However, I cannot seem to get it working. I'm pretty sure its nothing on your side and something with the way I'm setting it up.

I have the following:

      require_once("scripts/php/Markdown/parsedown.php");
      require_once("scripts/php/Markdown/SecureParsedown.php");

      use Aidantwoods\SecureParsedown\SecureParsedown;

      $parsedown = new SecureParsedown;
      $parsedown->setSafeMode(true);

And I get this error: Parse error: syntax error, unexpected 'use' (T_USE)

@aidantwoods
Copy link
Collaborator

aidantwoods commented Sep 29, 2017

From the error it looks like you're perhaps declaring this within a function in the surrounding context?

PHP's use is a little weird, and it can have three (thus far) different meanings depending on where it is used.

In the desired case, it is for importing – so you'd need to place the use statement outside of any classes and methods (more info)

Alternatively if that's not practical, just instantiate the class by using its fully qualified name, i.e. change new SecureParsedown; to new Aidantwoods\SecureParsedown\SecureParsedown; and remove the use line.

@embluk
Copy link

embluk commented Sep 29, 2017

Ohh yeah I had my previous code block within a wrapper class, inside a function.

It now works with using the fully qualified name.

Thanks for your rapid response and support! Keep up the good work!

@erusev
Copy link
Owner

erusev commented Sep 29, 2017

It's been a busy couple of months for me, I'll merge it as soon as I have some time to get my head around the code.

@jamesgraham
Copy link

jamesgraham commented Feb 25, 2018

Anyone using the roave/security-advisories will be unable to use this package as it's now marked unsafe.

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

Successfully merging a pull request may close this issue.

6 participants