Purge the developer tools of innerHTML: Editor - openDialog codemirror extension

RESOLVED FIXED in Firefox 28

Status

P2
normal
RESOLVED FIXED
5 years ago
3 months ago

People

(Reporter: mgoodwin, Assigned: anton)

Tracking

({sec-want})

unspecified
Firefox 28
x86_64
Linux
sec-want
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
CodeMirror manages to completely avoid use of innerHTML (yay!) but our openDialog extension doesn't. This is unfortunate as, whilst this is not exploitable at present, the use of innerHTML now is a common cause of vulnerability later.

See: https://mxr.mozilla.org/mozilla-central/source/browser/devtools/sourceeditor/codemirror/dialog/dialog.js#13
(Reporter)

Updated

5 years ago
Assignee: nobody → anton
(Assignee)

Updated

5 years ago
Priority: -- → P2
(Assignee)

Comment 1

5 years ago
Opened a pull request for the proposed change: https://github.com/marijnh/CodeMirror/pull/1963. Depending on the response, I'll either pull the latest version of CM3 into the tree or modify dialog addon and update READMEs. After that, it's super easy to update extensions that rely on this addon (only search and jump to line AFAIK).
(Assignee)

Updated

5 years ago
Blocks: 816756
(Assignee)

Comment 2

5 years ago
Created attachment 8335581 [details] [diff] [review]
WIP 1

This patch imports my patch to CodeMirror [1] and changes jumpToLine and search functionality to use DOM nodes instead of innerHTML. It doesn't touch replace dialogs because we don't use them.

[1] https://github.com/marijnh/CodeMirror/pull/1963
Attachment #8335581 - Flags: review?(fayearthur)
Comment on attachment 8335581 [details] [diff] [review]
WIP 1

Review of attachment 8335581 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/sourceeditor/codemirror/search/search.js
@@ +53,5 @@
> +      let txt = doc.createTextNode(cm.l10n("findCmd.promptMessage"));
> +
> +      inp.type = "text";
> +      inp.style.width = "10em";
> +      inp.style.marginLeft = "1em";

marginLeft -> mozMarginStart

::: browser/devtools/sourceeditor/editor.js
@@ +627,5 @@
> +    let txt = doc.createTextNode(L10N.GetStringFromName("gotoLineCmd.promptTitle"));
> +
> +    inp.type = "text";
> +    inp.style.width = "10em";
> +    inp.style.marginLeft = "1em";

mozMarginStart
Attachment #8335581 - Flags: review?(fayearthur) → review+
(Assignee)

Updated

5 years ago
Whiteboard: [fixed-in-fx-team]
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/5bb4635154f9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28

Updated

3 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.