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.

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

feat: allow customizing css filter and establish modified defaults #97

Merged
merged 4 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
50 changes: 50 additions & 0 deletions src/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getDefaultCSSWhiteList } from "xss";
import { Sanitizer } from "./index";

// This file contains basic tests that validate the utility methods.
Expand Down Expand Up @@ -38,6 +39,20 @@ describe("Sanitizer", () => {
"radarscope"
];

function getCSSOptions() {
const cssWhiteList = getDefaultCSSWhiteList();
cssWhiteList["flex"] = true;
cssWhiteList["flex-basis"] = true;
cssWhiteList["flex-direction"] = true;
cssWhiteList["flex-flow"] = true;
cssWhiteList["flex-grow"] = true;
cssWhiteList["flex-shrink"] = true;
cssWhiteList["flex-wrap"] = true;
cssWhiteList["line-height"] = true;
cssWhiteList["overflow"] = true;
return { whiteList: cssWhiteList };
}

test("creates the Sanitizer object and extends options appropriately", () => {
// Test with no arguments
const sanitizer1 = new Sanitizer();
Expand All @@ -46,6 +61,7 @@ describe("Sanitizer", () => {
defaultSanitizer1.arcgisFilterOptions
);
defaultOptions1.whiteList = defaultSanitizer1.arcgisWhiteList;
defaultOptions1.css = getCSSOptions();
expect(sanitizer1.xssFilterOptions).toEqual(defaultOptions1);

// Extending the defaults
Expand All @@ -58,6 +74,7 @@ describe("Sanitizer", () => {
filterOptions2.whiteList = defaultSanitizer2.arcgisWhiteList;
filterOptions2.whiteList.blink = [];
filterOptions2.allowCommentTag = false;
filterOptions2.css = getCSSOptions();
expect(sanitizer2.xssFilterOptions).toEqual(filterOptions2);

// Passing an empty whitelist
Expand All @@ -68,11 +85,25 @@ describe("Sanitizer", () => {
defaultSanitizer3.arcgisFilterOptions
);
defaultOptions3.whiteList = defaultSanitizer3.arcgisWhiteList;
defaultOptions3.css = getCSSOptions();
expect(sanitizer3.xssFilterOptions).toEqual(defaultOptions3);

// Test overriding defaults
const sanitizer4 = new Sanitizer({ whiteList: { a: [] } });
expect(sanitizer4.xssFilterOptions).toEqual({ whiteList: { a: [] } });

// Extending the CSS defaults
const sanitizer5 = new Sanitizer({ css: { whiteList: { "line-height": false, "align-items": true } } }, true);
const defaultSanitizer5 = new Sanitizer();
const defaultOptions5 = Object.create(
defaultSanitizer5.arcgisFilterOptions
);
defaultOptions5.css = getCSSOptions();
defaultOptions5.css.whiteList["line-height"] = false;
defaultOptions5.css.whiteList["align-items"] = true;
expect((sanitizer5.xssFilterOptions.css as any).whiteList["line-height"]).toBeFalsy();
expect((sanitizer5.xssFilterOptions.css as any).whiteList["align-items"]).toBeTruthy();
expect(sanitizer5.xssFilterOptions).toEqual(defaultOptions5);
});

test("sanitizes a value", () => {
Expand Down Expand Up @@ -451,6 +482,7 @@ describe("Sanitizer", () => {
const strippedVideoSrc = "<video controls>";
const fontFace = `<font face="Arial">Text content</font>`;
const figure = `<figure style="background-color:blue;background-image:url("javascript:alert(\"xss\")";" onerror="alert(1)" onclick="javascript:alert(\"xss\")"><figcaption style="background-color:red;background-image:url("javascript:alert(\"xss\")";" onerror="alert(1)" onclick="javascript:alert(\"xss\")">Figure Caption</figcaption></figure>`;
const flexContainer = `<div style="flex:1 1 0%; flex-direction:column;">Flex container</div>`;
const elementsWithStyle = ["a", "img", "span", "div", "font", "table", "tr", "th", "td", "p", "dd", "dl", "dt", "h1", "h2", "h3", "h4", "h5", "h6", "sub", "sup"];

const sanitizer = new Sanitizer();
Expand All @@ -466,6 +498,7 @@ describe("Sanitizer", () => {
expect(sanitizer.sanitize(video)).toBe(video);
expect(sanitizer.sanitize(stripVideoSrc)).toBe(strippedVideoSrc);
expect(sanitizer.sanitize(fontFace)).toBe(fontFace);
expect(sanitizer.sanitize(flexContainer)).toBe(flexContainer);
expect(sanitizer.sanitize(figure)).toBe(
`<figure style="background-color:blue;"><figcaption style="background-color:red;">Figure Caption</figcaption></figure>`
);
Expand Down Expand Up @@ -585,4 +618,21 @@ describe("Sanitizer", () => {
expect(sanitizer.sanitize(includesAllowed)).toBe(sanitizedAllowed);
});

test("css overrides", () => {
const sanitizer = new Sanitizer();
const flexProperties = [
"flex:1 1 0%;",
"flex-basis:0.25rem;",
"flex-direction:row;",
"flex-flow:row wrap;",
"flex-grow:1;",
"flex-shrink:1;",
"flex-wrap:wrap;",
];
flexProperties.forEach((prop) => {
const value = `<div style="${prop}">Flex property test</div>`;
expect(sanitizer.sanitize(value)).toBe(value);
});
});

});
39 changes: 39 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ export interface IWhiteList extends XSS.IWhiteList {
source?: string[];
}

export interface ICSSFilter extends XSS.ICSSFilter {
whiteList: ICSSWhiteList;
}
export interface ICSSWhiteList {
[key: string]: boolean;
}

/** Options to apply to sanitize method */
export interface ISanitizeOptions {
/* Don't convert undefined to null */
Expand Down Expand Up @@ -120,6 +127,17 @@ export class Sanitizer {
"width",
],
};
private readonly arcgisCSSWhiteListOverrides: ICSSWhiteList = {
BlakeStearman marked this conversation as resolved.
Show resolved Hide resolved
"flex": true,
"flex-basis": true,
"flex-direction": true,
"flex-flow": true,
"flex-grow": true,
"flex-shrink": true,
"flex-wrap": true,
"line-height": true,
"overflow": true
};
public readonly allowedProtocols: string[] = [
"http",
"https",
Expand Down Expand Up @@ -186,15 +204,27 @@ export class Sanitizer {
// Override the defaults
xssFilterOptions = filterOptions;
} else if (filterOptions && extendDefaults) {
const cssWhiteList = this._getArcGISCSSWhiteList();

// Extend the defaults
xssFilterOptions = Object.create(this.arcgisFilterOptions);
xssFilterOptions.css = { whiteList: cssWhiteList };
Object.keys(filterOptions).forEach((key) => {
if (key === "whiteList") {
// Extend the whitelist by concatenating arrays
xssFilterOptions.whiteList = this._extendObjectOfArrays([
this.arcgisWhiteList,
filterOptions.whiteList || {},
]);
} else if (key === "css") {
const cssExtensions = (filterOptions.css as ICSSFilter).whiteList;
if (cssExtensions != null && filterOptions.css instanceof Object) {
Object.keys(cssExtensions).forEach(
(attr) =>
((xssFilterOptions.css as ICSSFilter).whiteList[attr] =
cssExtensions[attr])
);
}
} else {
xssFilterOptions[key] = filterOptions[key];
}
Expand All @@ -203,6 +233,7 @@ export class Sanitizer {
// Only use the defaults
xssFilterOptions = Object.create(this.arcgisFilterOptions);
xssFilterOptions.whiteList = this.arcgisWhiteList;
xssFilterOptions.css = { whiteList: this._getArcGISCSSWhiteList() };
}

this.xssFilterOptions = xssFilterOptions;
Expand Down Expand Up @@ -353,6 +384,14 @@ export class Sanitizer {
});
}

private _getArcGISCSSWhiteList() {
const cssWhiteList = xss.getDefaultCSSWhiteList();
Object.keys(this.arcgisCSSWhiteListOverrides).forEach((key) => {
BlakeStearman marked this conversation as resolved.
Show resolved Hide resolved
cssWhiteList[key] = this.arcgisCSSWhiteListOverrides[key];
});
return cssWhiteList;
}

/**
* Extends an object of arrays by by concatenating arrays of the same object
* keys. If the if the previous key's value is not an array, the next key's
Expand Down
Loading