1197206 - Validator misses innerHTML assigning

Status: RESOLVED WONTFIX

(addons.mozilla.org Graveyard :: Add-on Validation, defect)

Description

[Andreas Wagner [:TheOne] [use NI]]

https://addons.mozilla.org/en-US/firefox/files/browse/340566/file/chrome/content/lib/ts/player.js#L451

Comment 1

[Kris Maglione [:kmag]]

It didn't miss it, it just (incorrectly) thinks it's a static string, and the markup tester doesn't have any tests for <object> nodes.

Comment 2

[Kris Maglione [:kmag]]

This is, incidentally and unintentionally, fixed in my local branch:

Warning: Markup should not be passed to `innerHTML` dynamically.
Due to both security and performance concerns, innerHTML may not be set using dynamic values which have not been adequately sanitized. This can lead to security issues or fairly serious performance degradation.
        Tier:   3
        File:   chrome/content/lib/ts/player.js
        Line:   451
        Column: 34
        Context:
        > 
        > container.innerHTML = html;
        > embed = container.firstChild;

Comment 3

[Wil Clouser [:clouserw]]

What is your local branch?  You mean from another bug?  I'll assign this to you if you have a fix

Comment 4

[Wladimir Palant]

I filed bug 1209190 which might be a duplicate of this one - there is too little info here to tell.

Comment 5

[Wladimir Palant]

Also filed 1210498 which is another condition where assigning to innerHTML won't be recognized.

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Also https://addons.mozilla.org/en-US/firefox/files/browse/355248/file/data/tickethelper/tickethelper.lib.js#L2138 which looks similar to this bug.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

For those of you that can't access the unlisted add-ons, here is the snippet from my link:

btBuglistHtml = '<table>';
btBuglistHtml += ' <tbody>';
btBuglistHtml += '  <tr>';
btBuglistHtml += '   <td>' + MsgDepr.get('btd_buglistchange_in_list') + ':</td>';
btBuglistHtml += '   <td><input id="btBuglistShow" type="checkbox" name="in_list" /></td>';
btBuglistHtml += '  </tr>';
/* --- table truncated for brevity --- */
btBuglistHtml += ' </tbody>';
btBuglistHtml += '</table>';
btBuglistDiv = document.createElement('div');
btBuglistDiv.id = "btBuglist";
btBuglistDiv.className = this.project.getName() + " hidden";
btBuglistDiv.innerHTML = btBuglistHtml;

And here from Andreas' link (slightly modified for brevity):

// |container| is passed in through a function parameter
var html = "", clsid = "FAA285EB-EB55-47ff-84FF-0993CA2A41B5";
 
html += '<object classid="clsid:' + clsid + '" width="50%" height="100%">';
html += '<param name="autoplay" value="0" />';
html += '<param name="loop" value="0" />';
html += '<param name="bgcolor" value="' + bgColor + '" />';
html += '<param name="video-bgcolor" value="' + bgColor + '" />';
html += '<param name="fontcolor" value="' + fontColor + '" />';
html += '<param name="internalplaylist" value="' + internalPlaylist + '" />';
 
html += '<param name="fullscreencontrols" value="true" />';
html += '<param name="fscontrolsenable" value="1" />';
html += '<param name="nofscontrolsenable" value="1" />';
if(conf.liveStreamControls) {
    html += '<param name="defaultcontrolsforstream" value="1" />';
} else {
    html += '<param name="defaultcontrolsforstream" value="0" />';
}
html += '<param name="fscontrols" value="default" />';
html += '<param name="nofscontrols" value="default" />';
html += '<param name="nofscontrolsheight" value="36" />';
html += '</object>';
 
container.innerHTML = html;

Comment 8

[Wladimir Palant]

Thank you, both are the same as bug 1210498 then. Up to you which one you want to resolve as duplicate.

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Let's use this one since Kris has already mentioned he has a fix. What is the status on that btw?

Comment 11

[Sylvain Giroux [:Spoji]]

I got the same problem here on line 66 :
https://addons.mozilla.org/en-US/firefox/files/browse/361796/file/data/panel.js#L66