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.

Sign up for GitHub

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

Jump to bottom

Fix 8166 html tag validation #8176

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fix 8166 html tag validation #8176

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5476163
wip proposal still contains commented old code - tests are missing
pmario Apr 26, 2024
8f5d2d3
rename isTagNameSafe to makeTagNameSafe
pmario May 4, 2024
78ffa63
remove comments
pmario May 4, 2024
6172c7f
remove redundant space
pmario May 4, 2024
8345445
add htmlCustomPrimitives to the $tw.config object
pmario Jun 9, 2024
68ba4d5
remove tag-sanitation from element.js
pmario Jun 9, 2024
04a916d
WIP - add html-element sanitation to the new makeTagNameSafe function…
pmario Jun 9, 2024
8a04a09
simplify sanitation logic and fix inline docs
pmario Jun 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions core/modules/editor/engines/framed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,7 @@ function FramedEngine(options) {
this.iframeDoc.body.style.padding = "0";
this.widget.domNodes.push(this.iframeNode);
// Construct the textarea or input node
var tag = this.widget.editTag;
if($tw.config.htmlUnsafeElements.indexOf(tag) !== -1) {
tag = "input";
}
var tag = $tw.utils.makeTagNameSafe(this.widget.editTag, "input");
pmario marked this conversation as resolved.
Show resolved Hide resolved
this.domNode = this.iframeDoc.createElement(tag);
// Set the text
if(this.widget.editTag === "textarea") {
Expand Down Expand Up @@ -200,7 +197,7 @@ FramedEngine.prototype.handleFocusEvent = function(event) {
Handle a keydown event
*/
FramedEngine.prototype.handleKeydownEvent = function(event) {
if ($tw.keyboardManager.handleKeydownEvent(event, {onlyPriority: true})) {
if($tw.keyboardManager.handleKeydownEvent(event, {onlyPriority: true})) {
pmario marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

Expand Down
5 changes: 1 addition & 4 deletions core/modules/editor/engines/simple.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ function SimpleEngine(options) {
this.parentNode = options.parentNode;
this.nextSibling = options.nextSibling;
// Construct the textarea or input node
var tag = this.widget.editTag;
if($tw.config.htmlUnsafeElements.indexOf(tag) !== -1) {
tag = "input";
}
var tag = $tw.utils.makeTagNameSafe(this.widget.editTag,"input");
this.domNode = this.widget.document.createElement(tag);
// Set the text
if(this.widget.editTag === "textarea") {
Expand Down
25 changes: 20 additions & 5 deletions core/modules/utils/dom/dom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ exports.copyToClipboard = function(text,options) {
var succeeded = false;
try {
succeeded = document.execCommand("copy");
} catch (err) {
} catch(err) {
}
if(!options.doNotNotify) {
$tw.notifier.display(succeeded ? "$:/language/Notifications/CopiedToClipboard/Succeeded" : "$:/language/Notifications/CopiedToClipboard/Failed");
Expand All @@ -306,8 +306,8 @@ Collect DOM variables
*/
exports.collectDOMVariables = function(selectedNode,domNode,event) {
var variables = {},
selectedNodeRect,
domNodeRect;
selectedNodeRect,
domNodeRect;
if(selectedNode) {
$tw.utils.each(selectedNode.attributes,function(attribute) {
variables["dom-" + attribute.name] = attribute.value.toString();
Expand All @@ -324,7 +324,7 @@ exports.collectDOMVariables = function(selectedNode,domNode,event) {
variables["tv-popup-coords"] = Popup.buildCoordinates(Popup.coordinatePrefix.csOffsetParent,nodeRect);

var absRect = $tw.utils.extend({}, nodeRect);
for (var currentNode = selectedNode.offsetParent; currentNode; currentNode = currentNode.offsetParent) {
for(var currentNode = selectedNode.offsetParent; currentNode; currentNode = currentNode.offsetParent) {
absRect.left += currentNode.offsetLeft;
absRect.top += currentNode.offsetTop;
}
Expand Down Expand Up @@ -366,7 +366,7 @@ exports.collectDOMVariables = function(selectedNode,domNode,event) {
};

/*
Make sure the CSS selector is not invalid
Make sure the CSS selector is valid
*/
exports.querySelectorSafe = function(selector,baseElement) {
baseElement = baseElement || document;
Expand All @@ -386,4 +386,19 @@ exports.querySelectorAllSafe = function(selector,baseElement) {
}
};

/*
Make sure HTML tag names are valid
*/
exports.makeTagNameSafe = function(tag,defaultTag) {
defaultTag = defaultTag || "SPAN";
// This implements a check for standard DOM elements: https://html.spec.whatwg.org/#syntax-tag-name
// It does _not_ deal with Custom Elements: https://html.spec.whatwg.org/#valid-custom-element-name
pmario marked this conversation as resolved.
Show resolved Hide resolved
var regexp = /[a-zA-Z0-9]/g;
if(tag && tag.match(regexp) && $tw.config.htmlUnsafeElements.indexOf(tag) === -1) {
return tag;
}
return defaultTag;
};


})();
6 changes: 2 additions & 4 deletions core/modules/widgets/button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ ButtonWidget.prototype.render = function(parent,nextSibling) {
this.computeAttributes();
this.execute();
// Create element
if(this.buttonTag && $tw.config.htmlUnsafeElements.indexOf(this.buttonTag) === -1) {
tag = this.buttonTag;
}
tag = $tw.utils.makeTagNameSafe(this.buttonTag,tag)
domNode = this.document.createElement(tag);
this.domNode = domNode;
// Assign classes
Expand Down Expand Up @@ -74,7 +72,7 @@ ButtonWidget.prototype.render = function(parent,nextSibling) {
if(this["aria-label"]) {
domNode.setAttribute("aria-label",this["aria-label"]);
}
if (this.role) {
if(this.role) {
domNode.setAttribute("role", this.role);
}
if(this.popup || this.popupTitle) {
Expand Down
5 changes: 1 addition & 4 deletions core/modules/widgets/draggable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,7 @@ DraggableWidget.prototype.render = function(parent,nextSibling) {
// Execute our logic
this.execute();
// Sanitise the specified tag
tag = this.draggableTag;
if($tw.config.htmlUnsafeElements.indexOf(tag) !== -1) {
tag = "div";
}
tag = $tw.utils.makeTagNameSafe(this.draggableTag,"div");
// Create our element
domNode = this.document.createElement(tag);
// Assign classes
Expand Down
4 changes: 1 addition & 3 deletions core/modules/widgets/droppable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ DroppableWidget.prototype.render = function(parent,nextSibling) {
// Compute attributes and execute state
this.computeAttributes();
this.execute();
if(this.droppableTag && $tw.config.htmlUnsafeElements.indexOf(this.droppableTag) === -1) {
tag = this.droppableTag;
}
tag = $tw.utils.makeTagNameSafe(this.droppableTag,tag);
// Create element and assign classes
domNode = this.document.createElement(tag);
this.domNode = domNode;
Expand Down
4 changes: 1 addition & 3 deletions core/modules/widgets/element.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ ElementWidget.prototype.render = function(parent,nextSibling) {
this.computeAttributes();
// Neuter blacklisted elements
this.tag = this.parseTreeNode.tag;
if($tw.config.htmlUnsafeElements.indexOf(this.tag) !== -1) {
this.tag = "safe-" + this.tag;
}
this.tag = $tw.utils.makeTagNameSafe(this.tag, "safe" + this.tag);
pmario marked this conversation as resolved.
Show resolved Hide resolved
// Restrict tag name to digits, letts and dashes
this.tag = this.tag.replace(/[^0-9a-zA-Z\-]/mg,"");
// Default to a span
Expand Down
4 changes: 1 addition & 3 deletions core/modules/widgets/eventcatcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ EventWidget.prototype.render = function(parent,nextSibling) {
this.execute();
// Create element
var tag = this.parseTreeNode.isBlock ? "div" : "span";
if(this.elementTag && $tw.config.htmlUnsafeElements.indexOf(this.elementTag) === -1) {
tag = this.elementTag;
}
tag = $tw.utils.makeTagNameSafe(this.elementTag,tag)
var domNode = this.document.createElement(tag);
this.domNode = domNode;
// Assign classes
Expand Down
6 changes: 2 additions & 4 deletions core/modules/widgets/keyboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ KeyboardWidget.prototype.render = function(parent,nextSibling) {
this.computeAttributes();
this.execute();
var tag = this.parseTreeNode.isBlock ? "div" : "span";
if(this.tag && $tw.config.htmlUnsafeElements.indexOf(this.tag) === -1) {
tag = this.tag;
}
tag = $tw.utils.makeTagNameSafe(this.tag,tag);
// Create element
var domNode = this.document.createElement(tag);
// Assign classes
Expand All @@ -53,7 +51,7 @@ KeyboardWidget.prototype.render = function(parent,nextSibling) {
};

KeyboardWidget.prototype.handleChangeEvent = function(event) {
if ($tw.keyboardManager.handleKeydownEvent(event, {onlyPriority: true})) {
if($tw.keyboardManager.handleKeydownEvent(event, {onlyPriority: true})) {
return true;
}

Expand Down
4 changes: 1 addition & 3 deletions core/modules/widgets/link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ LinkWidget.prototype.renderLink = function(parent,nextSibling) {
var self = this;
// Sanitise the specified tag
var tag = this.linkTag;
if($tw.config.htmlUnsafeElements.indexOf(tag) !== -1) {
tag = "a";
}
tag = $tw.utils.makeTagNameSafe(tag,"a");
// Create our element
var namespace = this.getVariable("namespace",{defaultValue: "http://www.w3.org/1999/xhtml"}),
domNode = this.document.createElementNS(namespace,tag);
Expand Down
8 changes: 3 additions & 5 deletions core/modules/widgets/reveal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ RevealWidget.prototype.render = function(parent,nextSibling) {
this.computeAttributes();
this.execute();
var tag = this.parseTreeNode.isBlock ? "div" : "span";
if(this.revealTag && $tw.config.htmlUnsafeElements.indexOf(this.revealTag) === -1) {
tag = this.revealTag;
}
tag = $tw.utils.makeTagNameSafe(this.revealTag,tag);
var domNode = this.document.createElement(tag);
this.domNode = domNode;
this.assignDomNodeClasses();
Expand Down Expand Up @@ -96,9 +94,9 @@ RevealWidget.prototype.positionPopup = function(domNode) {
left = Math.max(0,left);
top = Math.max(0,top);
}
if (this.popup.absolute) {
if(this.popup.absolute) {
// Traverse the offsetParent chain and correct the offset to make it relative to the parent node.
for (var offsetParentDomNode = domNode.offsetParent; offsetParentDomNode; offsetParentDomNode = offsetParentDomNode.offsetParent) {
for(var offsetParentDomNode = domNode.offsetParent; offsetParentDomNode; offsetParentDomNode = offsetParentDomNode.offsetParent) {
left -= offsetParentDomNode.offsetLeft;
top -= offsetParentDomNode.offsetTop;
}
Expand Down
Loading