Skip to content

Commit

Permalink
All: Security: Fixed potential Arbitrary File Read via XSS
Browse files Browse the repository at this point in the history
  • Loading branch information
laurent22 committed Feb 13, 2020
1 parent 06d807d commit 3db47b5
Show file tree
Hide file tree
Showing 24 changed files with 437 additions and 98 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,4 @@ ElectronClient/app/gui/ShareNoteDialog.js
ReactNativeClient/lib/JoplinServerApi.js
ReactNativeClient/PluginAssetsLoader.js
ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/mermaid.js
ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/sanitize_html.js
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,4 @@ ElectronClient/app/gui/ShareNoteDialog.js
ReactNativeClient/lib/JoplinServerApi.js
ReactNativeClient/PluginAssetsLoader.js
ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/mermaid.js
ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/sanitize_html.js
14 changes: 14 additions & 0 deletions CliClient/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion CliClient/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"markdown-it-toc-done-right": "^4.1.0",
"md5": "^2.2.1",
"md5-file": "^4.0.0",
"memory-cache": "^0.2.0",
"mime": "^2.0.3",
"moment": "^2.24.0",
"multiparty": "^4.2.1",
Expand Down Expand Up @@ -104,7 +105,8 @@
"valid-url": "^1.0.9",
"word-wrap": "^1.2.3",
"xml2js": "^0.4.19",
"yargs-parser": "^7.0.0"
"yargs-parser": "^7.0.0",
"node-html-parser": "^1.2.4"
},
"devDependencies": {
"jasmine": "^3.5.0"
Expand Down
79 changes: 79 additions & 0 deletions CliClient/tests/HtmlToHtml.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/* eslint-disable no-unused-vars */

require('app-module-path').addPath(__dirname);

const os = require('os');
const { time } = require('lib/time-utils.js');
const { filename } = require('lib/path-utils.js');
const { asyncTest, fileContentEqual, setupDatabase, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync } = require('test-utils.js');
const Folder = require('lib/models/Folder.js');
const Note = require('lib/models/Note.js');
const BaseModel = require('lib/BaseModel.js');
const { shim } = require('lib/shim');
const HtmlToHtml = require('lib/joplin-renderer/HtmlToHtml');
const { enexXmlToMd } = require('lib/import-enex-md-gen.js');

jasmine.DEFAULT_TIMEOUT_INTERVAL = 60 * 60 * 1000; // Can run for a while since everything is in the same test unit

process.on('unhandledRejection', (reason, p) => {
console.log('Unhandled Rejection at: Promise', p, 'reason:', reason);
});

describe('HtmlToHtml', function() {

beforeEach(async (done) => {
await setupDatabaseAndSynchronizer(1);
await switchClient(1);
done();
});

it('should convert from Html to Html', asyncTest(async () => {
const basePath = `${__dirname}/html_to_html`;
const files = await shim.fsDriver().readDirStats(basePath);
const htmlToHtml = new HtmlToHtml();

for (let i = 0; i < files.length; i++) {
const htmlSourceFilename = files[i].path;
if (htmlSourceFilename.indexOf('.src.html') < 0) continue;

const htmlSourceFilePath = `${basePath}/${htmlSourceFilename}`;
const htmlDestPath = `${basePath}/${filename(filename(htmlSourceFilePath))}.dest.html`;

// if (htmlSourceFilename !== 'table_with_header.html') continue;

const htmlToHtmlOptions = {
bodyOnly: true,
};

const sourceHtml = await shim.fsDriver().readFile(htmlSourceFilePath);
let expectedHtml = await shim.fsDriver().readFile(htmlDestPath);

const result = await htmlToHtml.render(sourceHtml, null, htmlToHtmlOptions);
let actualHtml = result.html;

if (os.EOL === '\r\n') {
expectedHtml = expectedHtml.replace(/\r\n/g, '\n');
actualHtml = actualHtml.replace(/\r\n/g, '\n');
}

if (actualHtml !== expectedHtml) {
console.info('');
console.info(`Error converting file: ${htmlSourceFilename}`);
console.info('--------------------------------- Got:');
console.info(actualHtml);
console.info('--------------------------------- Raw:');
console.info(actualHtml.split('\n'));
console.info('--------------------------------- Expected:');
console.info(expectedHtml.split('\n'));
console.info('--------------------------------------------');
console.info('');

expect(false).toBe(true);
// return;
} else {
expect(true).toBe(true);
}
}
}));

});
79 changes: 79 additions & 0 deletions CliClient/tests/MdToHtml.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/* eslint-disable no-unused-vars */

require('app-module-path').addPath(__dirname);

const os = require('os');
const { time } = require('lib/time-utils.js');
const { filename } = require('lib/path-utils.js');
const { asyncTest, fileContentEqual, setupDatabase, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync } = require('test-utils.js');
const Folder = require('lib/models/Folder.js');
const Note = require('lib/models/Note.js');
const BaseModel = require('lib/BaseModel.js');
const { shim } = require('lib/shim');
const MdToHtml = require('lib/joplin-renderer/MdToHtml');
const { enexXmlToMd } = require('lib/import-enex-md-gen.js');

jasmine.DEFAULT_TIMEOUT_INTERVAL = 60 * 60 * 1000; // Can run for a while since everything is in the same test unit

process.on('unhandledRejection', (reason, p) => {
console.log('Unhandled Rejection at: Promise', p, 'reason:', reason);
});

describe('MdToHtml', function() {

beforeEach(async (done) => {
await setupDatabaseAndSynchronizer(1);
await switchClient(1);
done();
});

it('should convert from Markdown to Html', asyncTest(async () => {
const basePath = `${__dirname}/md_to_html`;
const files = await shim.fsDriver().readDirStats(basePath);
const mdToHtml = new MdToHtml();

for (let i = 0; i < files.length; i++) {
const mdFilename = files[i].path;
if (mdFilename.indexOf('.md') < 0) continue;

const mdFilePath = `${basePath}/${mdFilename}`;
const htmlPath = `${basePath}/${filename(mdFilePath)}.html`;

// if (mdFilename !== 'table_with_header.html') continue;

const mdToHtmlOptions = {
bodyOnly: true,
};

const markdown = await shim.fsDriver().readFile(mdFilePath);
let expectedHtml = await shim.fsDriver().readFile(htmlPath);

const result = await mdToHtml.render(markdown, null, mdToHtmlOptions);
let actualHtml = result.html;

if (os.EOL === '\r\n') {
expectedHtml = expectedHtml.replace(/\r\n/g, '\n');
actualHtml = actualHtml.replace(/\r\n/g, '\n');
}

if (actualHtml !== expectedHtml) {
console.info('');
console.info(`Error converting file: ${mdFilename}`);
console.info('--------------------------------- Got:');
console.info(actualHtml);
console.info('--------------------------------- Raw:');
console.info(actualHtml.split('\n'));
console.info('--------------------------------- Expected:');
console.info(expectedHtml.split('\n'));
console.info('--------------------------------------------');
console.info('');

expect(false).toBe(true);
// return;
} else {
expect(true).toBe(true);
}
}
}));

});
2 changes: 2 additions & 0 deletions CliClient/tests/html_to_html/sanitize.dest.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<img src onerror="" />
<img src onerror="" />
3 changes: 3 additions & 0 deletions CliClient/tests/html_to_html/sanitize.src.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<img src="" onerror="alert('ohno')"/>
<img src=""
onerror="alert('ohno')"/>
2 changes: 2 additions & 0 deletions CliClient/tests/md_to_html/sanitize.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<img src onerror="" />
<img src onerror="" />
3 changes: 3 additions & 0 deletions CliClient/tests/md_to_html/sanitize.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<img src="" onerror="alert('ohno')"/>
<img src=""
onerror="alert('ohno')"/>
2 changes: 1 addition & 1 deletion Clipper/joplin-webclipper/content_scripts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
function absoluteUrl(url) {
if (!url) return url;
const protocol = url.toLowerCase().split(':')[0];
if (['http', 'https', 'file'].indexOf(protocol) >= 0) return url;
if (['http', 'https', 'file', 'data'].indexOf(protocol) >= 0) return url;

if (url.indexOf('//') === 0) {
return location.protocol + url;
Expand Down
Loading

0 comments on commit 3db47b5

Please sign in to comment.