Repository: Khan/KaTeX
Pull request #1440 (2018-06-19 02:07:13)

Allow only allowed protocols in \href
* Added `allowedProtocols` settings

codecov[bot]: # [Codecov](https://codecov.io/gh/Khan/KaTeX/pull/1440?src=pr&el=h1) Report
> Merging [#1440](https://codecov.io/gh/Khan/KaTeX/pull/1440?src=pr&el=desc) into [master](https://codecov.io/gh/Khan/KaTeX/commit/3f1b13e352408f7782e161f7c15b36e560c242b9?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `100%`.


[![Impacted file tree graph](https://codecov.io/gh/Khan/KaTeX/pull/1440/graphs/tree.svg?height=150&width=650&token=vfDnj2P77o&src=pr)](https://codecov.io/gh/Khan/KaTeX/pull/1440?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1440      +/-   ##
==========================================
+ Coverage    82.5%   82.52%   +0.02%     
==========================================
  Files          77       77              
  Lines        4390     4396       +6     
  Branches      767      769       +2     
==========================================
+ Hits         3622     3628       +6     
  Misses        665      665              
  Partials      103      103
```


| [Impacted Files](https://codecov.io/gh/Khan/KaTeX/pull/1440?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/Parser.js](https://codecov.io/gh/Khan/KaTeX/pull/1440/diff?src=pr&el=tree#diff-c3JjL1BhcnNlci5qcw==) | `94.47% <100%> (+0.07%)` | :arrow_up: |
| [src/Settings.js](https://codecov.io/gh/Khan/KaTeX/pull/1440/diff?src=pr&el=tree#diff-c3JjL1NldHRpbmdzLmpz) | `73.52% <100%> (+0.8%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Khan/KaTeX/pull/1440?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Khan/KaTeX/pull/1440?src=pr&el=footer). Last update [3f1b13e...dcb885c](https://codecov.io/gh/Khan/KaTeX/pull/1440?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ylemkimon: @edemaine Thank you for the review! I've added boolean argument to allow or disallow all protocols. I'm not sure about the function.
kevinbarabash: One alternative solution might be to allow `[ "*" ]` to represent all protocols.  This is going to get more interesting if we ever support embedding images.
edemaine: @kevinbarabash That might be considered more standard. I could go either way. @ylemkimon Thoughts?
ylemkimon: I think `["*"]` is more consistent. Changed to `["*"]`.
edemaine: Looks great!

Patch:
diff --git a/README.md b/README.md
index 6ffedb89..571f9898 100644
--- a/README.md
+++ b/README.md
@@ -55,9 +55,10 @@ Make sure to include the CSS and font files, but there is no need to include the
 Any HTML generated by KaTeX *should* be safe from `<script>` or other code
 injection attacks.
 (See `maxSize` below for preventing large width/height visual affronts,
-and see `maxExpand` below for preventing infinite macro loop attacks.)
+see `maxExpand` below for preventing infinite macro loop attacks, and
+see `allowedProtocols` below for preventing certain protocols in `\href`)
 Of course, it is always a good idea to sanitize the HTML, though you will need
-a rather generous whitelist (including some of SVG and MathML) to support 
+a rather generous whitelist (including some of SVG and MathML) to support
 all of KaTeX.
 
 #### Handling errors
@@ -85,6 +86,7 @@ You can provide an object of options as the last argument to `katex.render` and
 - `colorIsTextColor`: `boolean`. If `true`, `\color` will work like LaTeX's `\textcolor`, and take two arguments (e.g., `\color{blue}{hello}`), which restores the old behavior of KaTeX (pre-0.8.0). If `false` (the default), `\color` will work like LaTeX's `\color`, and take one argument (e.g., `\color{blue}hello`).  In both cases, `\textcolor` works as in LaTeX (e.g., `\textcolor{blue}{hello}`).
 - `maxSize`: `number`. All user-specified sizes, e.g. in `\rule{500em}{500em}`, will be capped to `maxSize` ems. If set to `Infinity` (the default), users can make elements and spaces arbitrarily large.
 - `maxExpand`: `number`. Limit the number of macro expansions to the specified number, to prevent e.g. infinite macro loops. If set to `Infinity`, the macro expander will try to fully expand as in LaTeX. (default: 1000)
+- `allowedProtocols`: `string[]`. Allowed protocols in `\href`. Use `_relative` to allow relative urls, and `*` to allow all protocols. (default: `["http", "https", "mailto", "_relative"]`)
 - `strict`: `boolean` or `string` or `function` (default: `"warn"`). If `false` or `"ignore`", allow features that make writing LaTeX convenient but are not actually supported by (Xe)LaTeX (similar to MathJax). If `true` or `"error"` (LaTeX faithfulness mode), throw an error for any such transgressions. If `"warn"` (the default), warn about such behavior via `console.warn`. Provide a custom function `handler(errorCode, errorMsg, token)` to customize behavior depending on the type of transgression (summarized by the string code `errorCode` and detailed in `errorMsg`); this function can also return `"ignore"`, `"error"`, or `"warn"` to use a built-in behavior.  A list of such features and their `errorCode`s:
   - `"unknownSymbol"`: Use of unknown Unicode symbol, which will likely also
     lead to warnings about missing character metrics, and layouts may be
diff --git a/src/Parser.js b/src/Parser.js
index 31fa2859..5760ab25 100644
--- a/src/Parser.js
+++ b/src/Parser.js
@@ -9,6 +9,7 @@ import {validUnit} from "./units";
 import {supportedCodepoint} from "./unicodeScripts";
 import unicodeAccents from "./unicodeAccents";
 import unicodeSymbols from "./unicodeSymbols";
+import utils from "./utils";
 import ParseNode, {assertNodeType, checkNodeType} from "./ParseNode";
 import ParseError from "./ParseError";
 import {combiningDiacriticalMarksEndRegex} from "./Lexer.js";
@@ -803,6 +804,12 @@ export default class Parser {
         // and keep them as-is. Some browser will replace backslashes with
         // forward slashes.
         const url = raw.replace(/\\([#$%&~_^{}])/g, '$1');
+        const protocol = /^\s*([^\\/#]*?)(?::|&#0*58|&#x0*3a)/i.exec(url);
+        const allowed = this.settings.allowedProtocols;
+        if (!utils.contains(allowed,  "*") && !utils.contains(allowed,
+                protocol != null ? protocol[1] : "_relative")) {
+            throw new ParseError('Not allowed \\href protocol', res);
+        }
         return newArgument(new ParseNode("url", {
             type: "url",
             value: url,
diff --git a/src/Settings.js b/src/Settings.js
index 493e9eee..0770bc1f 100644
--- a/src/Settings.js
+++ b/src/Settings.js
@@ -25,6 +25,7 @@ export type SettingsOptions = {
     strict?: boolean | "ignore" | "warn" | "error" | StrictFunction;
     maxSize?: number;
     maxExpand?: number;
+    allowedProtocols?: string[];
 };
 
 /**
@@ -46,6 +47,7 @@ class Settings {
     strict: boolean | "ignore" | "warn" | "error" | StrictFunction;
     maxSize: number;
     maxExpand: number;
+    allowedProtocols: string[];
 
     constructor(options: SettingsOptions) {
         // allow null options
@@ -58,6 +60,8 @@ class Settings {
         this.strict = utils.deflt(options.strict, "warn");
         this.maxSize = Math.max(0, utils.deflt(options.maxSize, Infinity));
         this.maxExpand = Math.max(0, utils.deflt(options.maxExpand, 1000));
+        this.allowedProtocols = utils.deflt(options.allowedProtocols,
+            ["http", "https", "mailto", "_relative"]);
     }
 
     /**


Test patch:
diff --git a/test/katex-spec.js b/test/katex-spec.js
index bedea36c..8071f03f 100644
--- a/test/katex-spec.js
+++ b/test/katex-spec.js
@@ -2468,6 +2468,23 @@ describe("An href command", function() {
         const markup = katex.renderToString("\\href{http://example.com/}{example here}");
         expect(markup).toContain("<a href=\"http://example.com/\">");
     });
+
+    it("should allow protocols in allowedProtocols", function() {
+        expect("\\href{relative}{foo}").toParse();
+        expect("\\href{ftp://x}{foo}").toParse(new Settings({
+            allowedProtocols: ["ftp"],
+        }));
+        expect("\\href{ftp://x}{foo}").toParse(new Settings({
+            allowedProtocols: ["*"],
+        }));
+    });
+
+    it("should not allow protocols not in allowedProtocols", function() {
+        expect("\\href{javascript:alert('x')}{foo}").toNotParse();
+        expect("\\href{relative}{foo}").toNotParse(new Settings({
+            allowedProtocols: [],
+        }));
+    });
 });
 
 describe("A parser that does not throw on unsupported commands", function() {