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

Unsafe HTML hijacking #269

Closed
gdh1995 opened this issue Apr 15, 2019 · 0 comments
Closed

Unsafe HTML hijacking #269

gdh1995 opened this issue Apr 15, 2019 · 0 comments

Comments

@gdh1995
Copy link
Contributor

gdh1995 commented Apr 15, 2019

Describe the bug

Viewer.js create its image list using ".alt" directly, but the "alt" attribute may include '"' characters and then cause potential hijacking:

const alt = image.alt || getImageNameFromURL(src);
let { url } = options;
if (isString(url)) {
url = image.getAttribute(url);
} else if (isFunction(url)) {
url = url.call(this, image);
}
if (src || url) {
items.push('<li>'
+ '<img'
+ ` src="${src || url}"`
+ ' role="button"'
+ ' data-viewer-action="view"'
+ ` data-index="${i}"`
+ ` data-original-url="${url || src}"`
+ ` alt="${alt}"`
+ '>'
+ '</li>');

I think an escaping of "alt" is needed here, and it's also beneficial to escape "src" and "url".

To Reproduce
Steps to reproduce the behavior:

  1. create an <img> and set `image.alt = 'a">nihao'
  2. activate Viewer.js on the image element
  3. in the .viewer-list DOM node, there will be a text node of nihao">

Expected behavior

The "alt" attribute can be a real a">nihao.

Screenshots

image

Desktop:

  • OS: Windows 10 x64
  • Browser: chrome 73
  • Version: 73, stable x64

Additional context

I have a Chrome extension Vimium C and it provides a command "LinkHints.activateModeToOpenImage" to open a new tab to display an arbitrary image. So I've used Viewer.js for years to provide moving and zooming functionalities.

Therefore, in my use case, all of image source URLs and "alt" text are from common web pages - they should not be "trusted" to be reliable and safe to create HTML. I once used JavaScript to create HTMLImageElement and then set ".alt = ...", but today I find Viewer.js does not. I think this is a bug causing potential HTML hijacking.

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

No branches or pull requests

1 participant