{"action":"created","issue":{"url":"https://api.github.com/repos/erusev/parsedown/issues/495","repository_url":"https://api.github.com/repos/erusev/parsedown","labels_url":"https://api.github.com/repos/erusev/parsedown/issues/495/labels{/name}","comments_url":"https://api.github.com/repos/erusev/parsedown/issues/495/comments","events_url":"https://api.github.com/repos/erusev/parsedown/issues/495/events","html_url":"https://github.com/erusev/parsedown/pull/495","id":225375951,"number":495,"title":"Prevent various XSS attacks [rebase and update of #276]","user":{"login":"aidantwoods","id":3288888,"avatar_url":"https://avatars1.githubusercontent.com/u/3288888?v=4","gravatar_id":"","url":"https://api.github.com/users/aidantwoods","html_url":"https://github.com/aidantwoods","followers_url":"https://api.github.com/users/aidantwoods/followers","following_url":"https://api.github.com/users/aidantwoods/following{/other_user}","gists_url":"https://api.github.com/users/aidantwoods/gists{/gist_id}","starred_url":"https://api.github.com/users/aidantwoods/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aidantwoods/subscriptions","organizations_url":"https://api.github.com/users/aidantwoods/orgs","repos_url":"https://api.github.com/users/aidantwoods/repos","events_url":"https://api.github.com/users/aidantwoods/events{/privacy}","received_events_url":"https://api.github.com/users/aidantwoods/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":38,"created_at":"2017-05-01T02:47:24Z","updated_at":"2017-10-31T10:30:21Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/erusev/parsedown/pulls/495","html_url":"https://github.com/erusev/parsedown/pull/495","diff_url":"https://github.com/erusev/parsedown/pull/495.diff","patch_url":"https://github.com/erusev/parsedown/pull/495.patch"},"body":"I've picked up on the work started over at https://github.com/erusev/parsedown/pull/276 and rebased on `erusev/master`.\r\n\r\nSince this is rebased on master, I can't point at PR at `naNuke/master` without running into the merge conflicts that I've already resolved manually.\r\n\r\nI've implemented what I suggested [earlier](https://github.com/erusev/parsedown/pull/276#issuecomment-298244798) so that all attributes are properly encoded (and not just the specific ones we remember).\r\n\r\nI've also added some tests, so @erusev's concern here should hopefully now be resolved, albeit a year later 😉 \r\nhttps://github.com/erusev/parsedown/pull/276#issuecomment-178577968\r\n> @malas one reason is the lack of tests\r\n\r\n~~One thing to note is that all this can be circumvented if you forget to turn on `$Parsedown->setMarkupEscaped(true);`\r\n(which is off by default) as you could just write a script tag manually for xss (even though the attributes and link destinations will be safe). So let's all remember to enable this setting 😉~~\r\n\r\nAttributes are now always escaped properly (this speaks to just outputting things correctly), but link based XSS or XSS from writing plain old script tags will only be prevented only if the new `setSafeMode` is enabled.\r\n\r\n```php\r\n$Parsedown->setSafeMode(true);\r\n```\r\n\r\n---\r\n\r\nCloses #161\r\nCloses #497\r\nCloses #276\r\nCloses #403"},"comment":{"url":"https://api.github.com/repos/erusev/parsedown/issues/comments/340721799","html_url":"https://github.com/erusev/parsedown/pull/495#issuecomment-340721799","issue_url":"https://api.github.com/repos/erusev/parsedown/issues/495","id":340721799,"user":{"login":"aidantwoods","id":3288888,"avatar_url":"https://avatars1.githubusercontent.com/u/3288888?v=4","gravatar_id":"","url":"https://api.github.com/users/aidantwoods","html_url":"https://github.com/aidantwoods","followers_url":"https://api.github.com/users/aidantwoods/followers","following_url":"https://api.github.com/users/aidantwoods/following{/other_user}","gists_url":"https://api.github.com/users/aidantwoods/gists{/gist_id}","starred_url":"https://api.github.com/users/aidantwoods/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aidantwoods/subscriptions","organizations_url":"https://api.github.com/users/aidantwoods/orgs","repos_url":"https://api.github.com/users/aidantwoods/repos","events_url":"https://api.github.com/users/aidantwoods/events{/privacy}","received_events_url":"https://api.github.com/users/aidantwoods/received_events","type":"User","site_admin":false},"created_at":"2017-10-31T10:30:21Z","updated_at":"2017-10-31T10:30:21Z","author_association":"CONTRIBUTOR","body":"Just to echo @h4ckninja's words here:\r\n> waiting on proven security, while leaving a library vulnerable, hurts users. I use this library for my projects because I like it. But leaving a parser to be \"just a parser\" is, honestly, dangerous. I've given a couple of examples above when secure design principles aren't followed.\r\n\r\nGenuinely every day that this goes unfixed in core, users apps remain vulnerable. Despite the (recently added) note in the README, the Wiki has read the following [since 2015](https://github.com/erusev/parsedown/wiki/Tutorial:-Get-Started/f9d881da38d8ef737ffc0c0a140b124ba3367904):\r\n> ```php\r\n> echo Parsedown::instance()\r\n>    ->setMarkupEscaped(true) # escapes markup (HTML)\r\n>    ->text(\"<div><strong>*Some text*</strong></div>\");\r\n> \r\n> # Output:\r\n> # <p>&lt;div&gt;&lt;strong&gt;<em>Some text</em>&lt;/strong&gt;&lt;/div&gt;</p>\r\n> ```\r\n\r\nGiving a strong impression that the library is capable of escaping HTML. I mean, it's literally what it says – **the fact that it doesn't properly is an implementation detail *and a bug*** (there are other ways to achieve xss that this PR also solves that wouldn't be tackled by just escaping HTML, but that's not really the point).\r\nThere are just a hair over 16,000 code results for \"parsedown setMarkupEscaped\", and you don't have to look very far for repos that are relying on it for sanitising user input. I would encourage you to have a browse.\r\n\r\nhttps://github.com/search?utf8=%E2%9C%93&q=parsedown+setMarkupEscaped&type=Code\r\n\r\nThe point here is to try and drive home the point that, unfortunately best intentions aside, this issue really does affect users."}}
{"actor":{"display_login":"aidantwoods"}}