Skip to content

Commit

Permalink
[plugin-host] Path traversal + XSS fix
Browse files Browse the repository at this point in the history
The plugin host used to send unescaped responses via HTML, leading to
a potential XSS vulnerability. Also, no constraint was applied to the file
serving functionality, which means that you can get served any file on
the filesystem if you add enough `../` parts in the requested path,
walking past the plugin's root being served.

This commit prevents clients to request something past the plugin's
root, and also encodes the plugin id -requested by the client- in its response.

Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com>
  • Loading branch information
paul-marechal committed Jul 18, 2019
1 parent d4a15ad commit f22963f
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 10 deletions.
14 changes: 7 additions & 7 deletions packages/plugin-ext/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
"@theia/search-in-workspace": "^0.8.0",
"@theia/task": "^0.8.0",
"@theia/workspace": "^0.8.0",
"@types/escape-html": "^0.0.20",
"decompress": "^4.2.0",
"escape-html": "^1.0.3",
"getmac": "^1.4.6",
"jsonc-parser": "^2.0.2",
"lodash.clonedeep": "^4.5.0",
Expand All @@ -30,13 +32,11 @@
"publishConfig": {
"access": "public"
},
"theiaExtensions": [
{
"backend": "lib/plugin-ext-backend-module",
"backendElectron": "lib/plugin-ext-backend-electron-module",
"frontend": "lib/plugin-ext-frontend-module"
}
],
"theiaExtensions": [{
"backend": "lib/plugin-ext-backend-module",
"backendElectron": "lib/plugin-ext-backend-electron-module",
"frontend": "lib/plugin-ext-frontend-module"
}],
"keywords": [
"theia-extension"
],
Expand Down
7 changes: 5 additions & 2 deletions packages/plugin-ext/src/hosted/node/plugin-reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import * as path from 'path';
import * as fs from 'fs-extra';
import * as express from 'express';
import * as escape_html from 'escape-html';
import { ILogger } from '@theia/core';
import { inject, injectable, optional, multiInject } from 'inversify';
import { BackendApplicationContribution } from '@theia/core/lib/node/backend-application';
Expand Down Expand Up @@ -50,9 +51,11 @@ export class HostedPluginReader implements BackendApplicationContribution {
const localPath = this.pluginsIdsFiles.get(pluginId);
if (localPath) {
const fileToServe = path.join(localPath, filePath);
res.sendFile(fileToServe);
res.sendFile(fileToServe, { root: localPath }, (error: Error) => {
res.status(404).send(`No such file for plugin with id '${escape_html(pluginId)}'.`);
});
} else {
res.status(404).send("The plugin with id '" + pluginId + "' does not exist.");
res.status(404).send(`The plugin with id '${escape_html(pluginId)}' does not exist.`);
}
});
}
Expand Down
8 changes: 7 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@
version "3.5.1"
resolved "https://registry.yarnpkg.com/@types/diff/-/diff-3.5.1.tgz#30253f6e177564ad7da707b1ebe46d3eade71706"

"@types/escape-html@^0.0.20":
version "0.0.20"
resolved "https://registry.yarnpkg.com/@types/escape-html/-/escape-html-0.0.20.tgz#cae698714dd61ebee5ab3f2aeb9a34ba1011735a"
integrity sha512-6dhZJLbA7aOwkYB2GDGdIqJ20wmHnkDzaxV9PJXe7O02I2dSFTERzRB6JrX6cWKaS+VqhhY7cQUMCbO5kloFUw==

"@types/events@*":
version "1.2.0"
resolved "https://registry.yarnpkg.com/@types/events/-/events-1.2.0.tgz#81a6731ce4df43619e5c8c945383b3e62a89ea86"
Expand Down Expand Up @@ -3610,9 +3615,10 @@ es6-promisify@^5.0.0:
dependencies:
es6-promise "^4.0.3"

escape-html@~1.0.3:
escape-html@^1.0.3, escape-html@~1.0.3:
version "1.0.3"
resolved "https://registry.yarnpkg.com/escape-html/-/escape-html-1.0.3.tgz#0258eae4d3d0c0974de1c169188ef0051d1d1988"
integrity sha1-Aljq5NPQwJdN4cFpGI7wBR0dGYg=

escape-string-applescript@^2.0.0:
version "2.0.0"
Expand Down

0 comments on commit f22963f

Please sign in to comment.