{{ message }}
This repository was archived by the owner on Jul 6, 2026. It is now read-only.
Repository navigation
This repository was archived by the owner on Jul 6, 2026. It is now read-only.
gallery.cnf leads to XSS #295
Description
---
# Gallery configuration file
information:
description: This <script>alert(1)</script>is an **album description** which is only shown if there is no `description_link`
copyright: Copyright 2003-2015 [interfaSys sàrl](javascript:alert(1\)), Switzerland
inherit: yes
sorting:
type: date
order: des
inherit: yes
features:
external_shares: yes
native_svg: yesPersonally I'd advise to remove this info stuff as it can easily lead to further security issues while offering few advantages.
Activity
oparoz commentedon Sep 3, 2015
I'm not sure I understand. You're worried about drive-by exploits? Worse damage can be done via an installed app. A user has to have some trust into the links he follows.
LukasReschke commentedon Sep 3, 2015
This is bad since if I can execute JS here I can do anything I want such as changing your password, accessing files, etc…
oparoz commentedon Sep 3, 2015
Yeah, internal shares are a problem... I'll add an HTML sanitizer.
LukasReschke commentedon Sep 3, 2015
Sanitizers are evil. Please go with a sane markdown interpreter that has a whitelist of things it converts instead of some magic that might get bypassed ;)
LukasReschke commentedon Sep 3, 2015
Especially since the only thing you need here are URLs which can be easily done yourself and you just need to check if it starts with
httporhttps:)oparoz commentedon Sep 3, 2015
I've selected a robust one, developed by security experts:
https://github.com/cure53/DOMPurify
I need more than links. Descriptions are written in markdown.
There is one thing which worries me though
Btw, you're going to face the same issue when introducing markdown based descriptions in apps.
LukasReschke commentedon Sep 3, 2015
Come on. That's not a proper solution. There have been 2 bypasses this year and we can solve it so easily reliable.
No. What do you need exactly? It's maybe "strong", "cursive", "links" you can have this pretty secure by choosing a sane markdown implementation or writing one yourself.
No. We're not. It's quite easy. Just use a markdown parser with a sane whitelist. Everything else is not really something which we should do.
LukasReschke commentedon Sep 3, 2015
I think what would make sense is reducing the attack surface by thinking what we need in the description. It barely is the whole HTML feature-set but some stuff that we can easily implement securely to make everybody happy ;)
oparoz commentedon Sep 3, 2015
DOMPurify does offer a whitelist
var clean = DOMPurify.sanitize(dirty, {ALLOWED_TAGS: ['b']});but yes, if you send me the link to a sane markdown parser, I'll use it instead of parsing the same content twice :). I don't want to write my own parser and have to maintain it.
I need at least:
* lists, 1,2,3 lists, # headers , *italic*, **strong**, [link](http)and it needs to be on bower.LukasReschke commentedon Sep 3, 2015
Yes. But DOMPurify is more meant to act as second line of defense instead of making a known insecure thing more secure :-) - Having it in makes sense, yes, but not as first line of defense.
Sane markdown parsers in JS… Hard topic… A lot of them lack some security features in one regard or another.
What is a sane one is https://github.com/jgm/commonmark.js, I'd recommend using that one with the
safeoption and also run DOMPurify later over the output. That way we have a defense in depth which never hurts 😄Markdown Plus! 😄 - Just kidding ;-)
oparoz commentedon Sep 3, 2015
OK. The lib I'm using also has a sanitize option, but it's disabled by default:
https://github.com/chjj/marked#sanitize
I think it's too restrictive as it gets rid of lists and only keeps paragraphs, but if not, I'll ask you to take a look :)
Yeah, there are already a few of those on Github :D
LukasReschke commentedon Sep 3, 2015
Yup. I'd go with the commonmark.js one as it can handle such stuff even in sanitize mode.
oparoz commentedon Sep 13, 2015
Fix is in #308