Closed Bug 1197206 Opened 5 years ago Closed 3 years ago

Validator misses innerHTML assigning

Categories

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

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: TheOne, Unassigned)

References

Details

(Whiteboard: [ReviewTeam])

Flags: needinfo?(kmaglione+bmo)
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.
Flags: needinfo?(kmaglione+bmo)
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;
What is your local branch?  You mean from another bug?  I'll assign this to you if you have a fix
Assignee: nobody → kmaglione+bmo
I filed bug 1209190 which might be a duplicate of this one - there is too little info here to tell.
Also filed 1210498 which is another condition where assigning to innerHTML won't be recognized.
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;
Thank you, both are the same as bug 1210498 then. Up to you which one you want to resolve as duplicate.
Let's use this one since Kris has already mentioned he has a fix. What is the status on that btw?
Flags: needinfo?(kmaglione+bmo)
Duplicate of this bug: 1210498
Product: addons.mozilla.org → addons.mozilla.org Graveyard
Assignee: kmaglione+bmo → nobody
Flags: needinfo?(kmaglione+bmo)
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.