Skip to content

Fix toBBCode not sanitising HTML #1022

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

Merged
samclarke merged 1 commit into from
Oct 22, 2026
Merged

Fix toBBCode not sanitising HTML #1022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion src/formats/bbcode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2466,6 +2466,8 @@
return toBBCode(element, hasCodeParent);
};

var sanitize;

/**
* Initializer
* @private
Expand All @@ -2484,6 +2486,7 @@
// Add BBCode helper methods
this.toBBCode = base.toSource;
this.fromBBCode = base.toHtml;
sanitize = this.__UNSTABLE__sanitize;
};

/**
Expand Down Expand Up @@ -2521,7 +2524,7 @@
var container = context.createElement('div');
var parser = new BBCodeParser(base.opts.parserOptions);

container.innerHTML = html;
container.innerHTML = sanitize(html);
css(containerParent, 'visibility', 'hidden');
containerParent.appendChild(container);
context.body.appendChild(containerParent);
Expand Down
2 changes: 1 addition & 1 deletion src/formats/xhtml.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@

base.toSource = toSource.bind(null, false);

base.fragmentToSource = toSource.bind(null, true);;
base.fragmentToSource = toSource.bind(null, true);

/**
* Runs all converters for the specified tagName
Expand Down
3 changes: 3 additions & 0 deletions src/lib/SCEditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,9 @@ export default function SCEditor(original, userOptions) {
});
};

// eslint-disable-next-line camelcase
base.__UNSTABLE__sanitize = sanitize;

/**
* Creates the editor iframe and textarea
* @private
Expand Down
16 changes: 12 additions & 4 deletions tests/unit/formats/bbcode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import 'src/formats/bbcode.js';
QUnit.module('formats/bbcode', {
beforeEach: function () {
this.mockEditor = {
opts: $.extend({}, defaultOptions)
opts: $.extend({}, defaultOptions),
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};

this.format = new sceditor.formats.bbcode();
Expand Down Expand Up @@ -95,7 +97,9 @@ QUnit.test('From BBCode method as fragment', function (assert) {

QUnit.test('BBcode to HTML trim', function (assert) {
this.mockEditor = {
opts: $.extend({}, defaultOptions, { bbcodeTrim: true })
opts: $.extend({}, defaultOptions, { bbcodeTrim: true }),
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};

this.format = new sceditor.formats.bbcode();
Expand All @@ -122,7 +126,9 @@ QUnit.test('BBcode to HTML trim', function (assert) {

QUnit.test('HTML to BBCode trim', function (assert) {
this.mockEditor = {
opts: $.extend({}, defaultOptions, { bbcodeTrim: true })
opts: $.extend({}, defaultOptions, { bbcodeTrim: true }),
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};

this.format = new sceditor.formats.bbcode();
Expand Down Expand Up @@ -152,7 +158,9 @@ QUnit.test('HTML to BBCode trim', function (assert) {
QUnit.module('formats/bbcode - HTML to BBCode', {
beforeEach: function () {
this.mockEditor = {
opts: $.extend({}, defaultOptions)
opts: $.extend({}, defaultOptions),
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};

this.format = new sceditor.formats.bbcode();
Expand Down
56 changes: 42 additions & 14 deletions tests/unit/formats/bbcode/matching.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ QUnit.test('Should match only if all attributes match when strict matching', fun
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down Expand Up @@ -58,7 +60,9 @@ QUnit.test('Should match if any attributes match when not strict matching', func
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down Expand Up @@ -100,7 +104,9 @@ QUnit.test('Should default to matching if any attributes match', function (asser
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down Expand Up @@ -144,7 +150,9 @@ QUnit.test('Should match twice if tag and styles match', function (assert) {
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand All @@ -169,7 +177,9 @@ QUnit.test('Should only match if all styles match when strict matching', functio
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down Expand Up @@ -197,7 +207,9 @@ QUnit.test('Should match if any styles match when not strict matching', function
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down Expand Up @@ -230,7 +242,9 @@ QUnit.test('Should default to matching if any styles match', function (assert) {
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down Expand Up @@ -262,7 +276,9 @@ QUnit.test('Should support matching wildcard tags', function (assert) {
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand All @@ -289,7 +305,9 @@ QUnit.test('Should wildcard matching styles or attributes', function (assert) {
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down Expand Up @@ -318,7 +336,9 @@ QUnit.test('Should only match when styles and attributes match if strict matchin
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down Expand Up @@ -355,7 +375,9 @@ QUnit.test('Should match when any style or attribute matches if not strict match
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down Expand Up @@ -390,7 +412,9 @@ QUnit.test('Should support style attribute', function (assert) {
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand All @@ -414,7 +438,9 @@ QUnit.test('Should match null style only if style attribute is specified', funct
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down Expand Up @@ -449,7 +475,9 @@ QUnit.test('Should match styles first', function (assert) {
});

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
(new sceditor.formats.bbcode()).init.call(mockEditor);

Expand Down
4 changes: 3 additions & 1 deletion tests/unit/formats/bbcode/nesting.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ QUnit.test('inline bbcodes must be inside block ones', assert => {
);

var mockEditor = {
opts: defaultOptions
opts: defaultOptions,
// eslint-disable-next-line camelcase
__UNSTABLE__sanitize: x => x
};
var format = new sceditor.formats.bbcode;
format.init.call(mockEditor);
Expand Down
42 changes: 42 additions & 0 deletions tests/unit/lib/SCEditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,3 +658,45 @@ QUnit.test('Do not wrap whitespace text nodes', function (assert) {

assert.htmlEqual(body.innerHTML, testHtml);
});


QUnit.test('toBBCode() strips XSS', function (assert) {
var done = assert.async();

reloadEditor({
format: 'bbcode'
});

document.body.xss = function () {};

var observer = new MutationObserver(function (mutations) {
var hasImage = false;

mutations.forEach(function (mutation) {
Array.prototype.forEach.call(mutation.addedNodes, function (node) {
if (node.nodeType !== 1 || !node.querySelector) {
return;
}

var img = node.querySelector('img');
if (img) {
hasImage = true;
assert.notOk(img.hasAttribute('onerror'), 'onerror not stripped');
}
});
});

observer.disconnect();
delete document.body.xss;

assert.ok(hasImage);
done();
});

observer.observe(document.body, {
childList: true,
subtree: true
});

sceditor.toBBCode('<img src="/x.png" onerror="document.body.xss()">');
});
Loading