search

itgalaxy / favicons

Favicons generator for Node.js
MIT License
1.19k stars 164 forks source link

HTML escape special characters #316

Closed doublemix closed 3 years ago

doublemix commented 4 years ago

I noticed that the HTML generation functions directly interpolate user input such as appName and appShortName. This makes it dangerous if working with user submitted content. For example:

const config = {
    appName: '"><script>alert("hacked")</script><meta name="bogus" value="',
    // rest of config
}

The above allows abritrary injection into the html head. (The above will create an alert box if the generated html is included into a page, although anything is possible at this point.)

Also, since appName and appShortName is used in other places like the manifests, escaping beforehand is not really an option. Don't want &amp; showing up in output.

At the least, appName and appShortName need escaped in HTML. Color should be fine if format is validated. Paths can be url encoded before hand by developer.

alexander-akait commented 4 years ago

PR welcome, developers love do bad things

doublemix commented 4 years ago

Working on it. Any recommendations/procedures for contributing?

Plan to only escape app name in HTML. Do you think of the other HTML interpolations should be escaped, as well? Don't think it can hurt.

alexander-akait commented 4 years ago

Just send the PR, I will review and give advises