Escape user input (via query params) properly

Author: domenic

.eslintrc.json

@@ -177,7 +177,7 @@
     "lines-around-comment": "off",
     "lines-around-directive": "off",
     "max-depth": "off",
-    "max-len": ["error", 100, { "ignoreUrls": true }],
+    "max-len": ["error", 100, { "ignoreUrls": true, "ignoreTemplateLiterals": true }],
     "max-lines": "off",
     "max-nested-callbacks": "off",
     "max-params": "off",

__tests__/__snapshots__/get-user-status.js.snap

@@ -88,6 +88,17 @@ Object {
 }
 `;
 
+exports[`Individuals exist, but the user is not one of them; it is an XSS attempt 1`] = `
+Object {
+  "context": "Participation",
+  "description": "@<script>alert(1);</script> has not signed up to participate",
+  "isNothing": true,
+  "longDescription": "@&lt;script&gt;alert(1);&lt;/script&gt; does not appear to have signed the agreement, or be associated with an entity that has done so. Please sign it, or forward it to an authorized representative!",
+  "state": "pending",
+  "target_url": "https://participate.whatwg.org/agreement-status?user=<script>alert(1);</script>&repo=console",
+}
+`;
+
 exports[`Multiple entities, all not in this workstream 1`] = `
 Object {
   "context": "Participation",

__tests__/get-user-status.js

@@ -203,3 +203,11 @@ test("Individuals exist, but the user is not one of them", async () => {
   ]);
   expect(await getUserStatus("johndoetw", "console")).toMatchSnapshot();
 });
+
+test("Individuals exist, but the user is not one of them; it is an XSS attempt", async () => {
+  mockData.set("individual-public", [
+    individualData(["console"], true, { id: "janedoetw" }),
+    individualData(["console"], true, { id: "bobbosstw" })
+  ]);
+  expect(await getUserStatus("<script>alert(1);</script>", "console")).toMatchSnapshot();
+});

lib/get-user-status.js

@@ -4,6 +4,7 @@ const listify = require("listify");
 const config = require("../config.json");
 const gitHubAPI = require("./helpers/github.js").api;
 const jsonGitHubDatabase = require("./helpers/json-github-database.js");
+const html = require("escape-goat").escapeTag;
 
 const statusURLBase = (new URL(config.statusPath, config.serverURL)).href;
 
@@ -60,8 +61,7 @@ function statusIndividual(submitterGitHubID, repoName) {
     state: "success",
     isNothing: false,
     description: `@${submitterGitHubID} has signed up to participate as an individual`,
-    longDescription: `@${submitterGitHubID} has signed up to participate as an individual. All ` +
-                     `is well; contribute at will!`,
+    longDescription: html`@${submitterGitHubID} has signed up to participate as an individual. All is well; contribute at will!`,
     target_url: `${statusURLBase}?user=${submitterGitHubID}&repo=${repoName}`,
     context: "Participation"
   };
@@ -75,10 +75,7 @@ function statusIndividualNoWorkstreams(submitterGitHubID, repoName) {
     state: "pending",
     isNothing: false,
     description: `@${submitterGitHubID} participates as an individual, but not in this workstream`,
-    longDescription: `@${submitterGitHubID} has signed up to participate as an individual, but ` +
-                     `has not chosen to participate in the ${workstreamName} workstream. To ` +
-                     `amend your agreement, submit a pull request to <a href="https://github.com/` +
-                     `${repo}">${repo}</a>.`,
+    longDescription: html`@${submitterGitHubID} has signed up to participate as an individual, but has not chosen to participate in the ${workstreamName} workstream. To amend your agreement, submit a pull request to <a href="https://github.com/${repo}">${repo}</a>.`,
     target_url: `${statusURLBase}?user=${submitterGitHubID}&repo=${repoName}`,
     context: "Participation"
   };
@@ -89,8 +86,7 @@ function statusIndividualUnverified(submitterGitHubID, repoName) {
     state: "pending",
     isNothing: false,
     description: `@${submitterGitHubID} is not yet verified`,
-    longDescription: `@${submitterGitHubID} has signed up to participate as an individual, but ` +
-                     `has not yet been verified. Hold tight while we sort this out on our end!`,
+    longDescription: html`@${submitterGitHubID} has signed up to participate as an individual, but has not yet been verified. Hold tight while we sort this out on our end!`,
     target_url: `${statusURLBase}?user=${submitterGitHubID}&repo=${repoName}`,
     context: "Participation"
   };
@@ -103,10 +99,7 @@ function statusEntity(submitterGitHubID, repoName, entity) {
     state: "success",
     isNothing: false,
     description: `${submitterGitHubID} participates on behalf of ${entity.info.name}`,
-    longDescription: `@${submitterGitHubID} is part of the ${entity.info.gitHubOrganization} ` +
-                     `GitHub organization, associated with ${entity.info.name}, which has signed ` +
-                     `the agreement and participates in the ${workstreamName} workstream. All is ` +
-                     `well; contribute at will!`,
+    longDescription: html`@${submitterGitHubID} is part of the ${entity.info.gitHubOrganization} GitHub organization, associated with ${entity.info.name}, which has signed the agreement and participates in the ${workstreamName} workstream. All is well; contribute at will!`,
     target_url: `${statusURLBase}?user=${submitterGitHubID}&repo=${repoName}`,
     context: "Participation"
   };
@@ -119,9 +112,7 @@ function statusEntitiesUnverified(submitterGitHubID, repoName, entities) {
     state: "pending",
     isNothing: false,
     description: `@${submitterGitHubID} participates on behalf of an unverified entity`,
-    longDescription: `@${submitterGitHubID} is part of the ${orgs} GitHub ${word}, but ` +
-                     `${pronoun} ${haveVerb} not yet been verified. Hold tight while we sort ` +
-                     `this out on our end!`,
+    longDescription: html`@${submitterGitHubID} is part of the ${orgs} GitHub ${word}, but ${pronoun} ${haveVerb} not yet been verified. Hold tight while we sort this out on our end!`,
     target_url: `${statusURLBase}?user=${submitterGitHubID}&repo=${repoName}`,
     context: "Participation"
   };
@@ -135,10 +126,7 @@ function statusEntityNoWorkstreams(submitterGitHubID, repoName, entities) {
     state: "pending",
     isNothing: false,
     description: `@${submitterGitHubID} participates on behalf of a non-workstream entity`,
-    longDescription: `@${submitterGitHubID} is part of the ${orgs} GitHub ${word}, but ` +
-                     `${pronoun} ${doVerb} not participate in the ${workstreamName} workstream. ` +
-                     `Please have the contact for ${pronoun2} ${word} update their participation ` +
-                     `agreement.`,
+    longDescription: html`@${submitterGitHubID} is part of the ${orgs} GitHub ${word}, but ${pronoun} ${doVerb} not participate in the ${workstreamName} workstream. Please have the contact for ${pronoun2} ${word} update their participation agreement.`,
     target_url: `${statusURLBase}?user=${submitterGitHubID}&repo=${repoName}`,
     context: "Participation"
   };
@@ -149,9 +137,7 @@ function statusNothing(submitterGitHubID, repoName) {
     state: "pending",
     isNothing: true,
     description: `@${submitterGitHubID} has not signed up to participate`,
-    longDescription: `@${submitterGitHubID} does not appear to have signed the agreement, or be ` +
-                     `associated with an entity that has done so. Please sign it, or forward it ` +
-                     `to an authorized representative!`,
+    longDescription: html`@${submitterGitHubID} does not appear to have signed the agreement, or be associated with an entity that has done so. Please sign it, or forward it to an authorized representative!`,
     target_url: `${statusURLBase}?user=${submitterGitHubID}&repo=${repoName}`,
     context: "Participation"
   };

package-lock.json

@@ -13,6 +13,7 @@
   },
   "dependencies": {
     "@octokit/rest": "^15.9.1",
+    "escape-goat": "^1.3.0",
     "github-username-regex": "^1.0.0",
     "handlebars": "^4.0.10",
     "http-errors": "^1.6.3",