Steps to reproduce:

  1. Install Media: YouTube and required modules. Configure display. Import a video, view the page with the video.
  2. Go to Configure >> File types >> Manage file display for videos.
  3. Set the width of the YouTube Video display to 560">><script>alert('hi');</script>. (Same thing works for the height.)
  4. Reload the video page. An alertbox with 'hi' shows up.

This is mitigated by the fact, that the attacker must have permission to configure the file display. That + there is no stable release, yet => A security advisory is not nescessary. (Thank you, @Dave Reid.)

Proposed resolution:
Use #element_validate => array('element_validate_integer_positive') on the textfields.

CommentFileSizeAuthor
#7 media_youtube-validate-formatter-xss-1547420-7.patch2.45 KBRobW
#6 media_youtube-validate-width-height-xss-1547420-6.patch1.61 KBRobW
#4 media_youube-checkplain-1547420-4.patch620 bytesRobW

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

We should also add a check_plain() to mitigate any existing malicious settings.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Priority: Normal » Critical

This seems critical.

aaron’s picture

aaron commented
Issue tags: +D7 stable release blocker

D7 stable release blocker

RobW’s picture

RobW commented
StatusFileSize
new media_youube-checkplain-1547420-4.patch620 bytes

Whoops, missed that. Thanks for finding the security hole, and here's a patch.

RobW’s picture

RobW commented
Status: Active » Needs review
RobW’s picture

RobW commented
StatusFileSize
new media_youtube-validate-width-height-xss-1547420-6.patch1.61 KB

Forget the patch in #4. Niklas's suggestion is way better.

This patch checks if user input is a number or percentage, with or without a decimal, or "auto". It uses the regex /^(auto|([0-9]*(\.[0-9]+)?%?))$/

If the check fails, the form throws the error:

The value entered for @dimension is invalid. Please insert a unitless integer for pixels, a percent, or "auto". Note that percent and auto may not function correctly depending on the browser and doctype.

Html 4.x allows percentages and auto in the width and height attributes, and most browsers will use any valid css size (percent, px, em) even though the current spec says unitless numbers only. Percent is still pretty common. I was thinking of allowing px and ems through validation too, but that would make the error message really long and might encourage bad behavior. Suggestions are welcome on where to draw the line.

RobW’s picture

RobW commented
StatusFileSize
new media_youtube-validate-formatter-xss-1547420-7.patch2.45 KB

As long as I was in there I added validation to the .js API origin, the only other user-entered text in the formatter.

The regex to match is ^https?\:\/\/[a-zA-Z0-9\-\.]+\.([a-zA-Z]{2,4}){1,2}$. Aiming for just restrictive enough.

RobW’s picture

RobW commented
Status: Needs review » Fixed

I've tested this pretty thoroughly. Since it's a security issue, committing.

http://drupalcode.org/project/media_youtube.git/commit/4bd2f34

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Close the

    .