1
0
mirror of https://github.com/laurent22/joplin.git synced 2024-11-24 08:12:24 +02:00

Desktop, Cli: Security: Fixed a path traversal vulnerability in clipper server API that could allow an attacker to read or write an arbitrary file (CVE-2020-15844)

This commit is contained in:
Laurent Cozic 2020-07-24 00:45:15 +00:00
parent 4be02bc33c
commit d209d5036b
3 changed files with 26 additions and 14 deletions

View File

@ -157,6 +157,7 @@ class BaseModel {
}
if (!('isNew' in options)) options.isNew = 'auto';
if (!('autoTimestamp' in options)) options.autoTimestamp = true;
if (!('userSideValidation' in options)) options.userSideValidation = false;
return options;
}
@ -444,6 +445,17 @@ class BaseModel {
return query;
}
static userSideValidation(o) {
if (('id' in o) && !o.id.match(/^[a-f0-9]{32}$/)) {
throw new Error('Validation error: ID must a 32-characters lowercase hexadecimal string');
}
const timestamps = ['user_updated_time', 'user_created_time'];
for (const k of timestamps) {
if ((k in o) && (typeof o[k] !== 'number' || isNaN(o[k]) || o[k] < 0)) throw new Error('Validation error: user_updated_time and user_created_time must be numbers greater than 0');
}
}
static async save(o, options = null) {
// When saving, there's a mutex per model ID. This is because the model returned from this function
// is basically its input `o` (instead of being read from the database, for performance reasons).
@ -471,6 +483,10 @@ class BaseModel {
o = this.filter(o);
if (options.userSideValidation) {
this.userSideValidation(o);
}
let queries = [];
const saveQuery = this.saveQuery(o, options);
const modelId = saveQuery.id;
@ -487,15 +503,10 @@ class BaseModel {
await this.db().transactionExecBatch(queries);
o = Object.assign({}, o);
// eslint-disable-next-line require-atomic-updates
if (modelId) o.id = modelId;
// eslint-disable-next-line require-atomic-updates
if ('updated_time' in saveQuery.modObject) o.updated_time = saveQuery.modObject.updated_time;
// eslint-disable-next-line require-atomic-updates
if ('created_time' in saveQuery.modObject) o.created_time = saveQuery.modObject.created_time;
// eslint-disable-next-line require-atomic-updates
if ('user_updated_time' in saveQuery.modObject) o.user_updated_time = saveQuery.modObject.user_updated_time;
// eslint-disable-next-line require-atomic-updates
if ('user_created_time' in saveQuery.modObject) o.user_created_time = saveQuery.modObject.user_created_time;
o = this.addModelMd(o);

View File

@ -382,7 +382,7 @@ class Api {
if (!request.files.length) throw new ErrorBadRequest('Resource cannot be created without a file');
const filePath = request.files[0].path;
const defaultProps = request.bodyJson(this.readonlyProperties('POST'));
return shim.createResourceFromPath(filePath, defaultProps);
return shim.createResourceFromPath(filePath, defaultProps, { userSideValidation: true });
}
return this.defaultAction_(BaseModel.TYPE_RESOURCE, request, id, link);

View File

@ -145,6 +145,7 @@ function shimInit() {
shim.createResourceFromPath = async function(filePath, defaultProps = null, options = null) {
options = Object.assign({
resizeLargeImages: 'always', // 'always', 'ask' or 'never'
userSideValidation: false,
}, options);
const readChunk = require('read-chunk');
@ -159,7 +160,7 @@ function shimInit() {
const resourceId = defaultProps.id ? defaultProps.id : uuid.create();
let resource = Resource.new();
const resource = Resource.new();
resource.id = resourceId;
resource.mime = mimeUtils.fromFilename(filePath);
resource.title = basename(filePath);
@ -186,15 +187,13 @@ function shimInit() {
const ok = await handleResizeImage_(filePath, targetPath, resource.mime, options.resizeLargeImages);
if (!ok) return null;
} else {
// const stat = await shim.fsDriver().stat(filePath);
// if (stat.size >= 10000000) throw new Error('Resources larger than 10 MB are not currently supported as they may crash the mobile applications. The issue is being investigated and will be fixed at a later time.');
await fs.copy(filePath, targetPath, { overwrite: true });
}
if (defaultProps) {
resource = Object.assign({}, resource, defaultProps);
}
// While a whole object can be passed as defaultProps, we only just
// support the title and ID (used above). Any other prop should be
// derived from the provided file.
if ('title' in defaultProps) resource.title = defaultProps.title;
const itDoes = await shim.fsDriver().waitTillExists(targetPath);
if (!itDoes) throw new Error(`Resource file was not created: ${targetPath}`);
@ -202,7 +201,9 @@ function shimInit() {
const fileStat = await shim.fsDriver().stat(targetPath);
resource.size = fileStat.size;
return Resource.save(resource, { isNew: true });
const saveOptions = { isNew: true };
if (options.userSideValidation) saveOptions.userSideValidation = true;
return Resource.save(resource, saveOptions);
};
shim.attachFileToNoteBody = async function(noteBody, filePath, position = null, options = null) {