From a9e496078261dfd6b9aee178e4ed044ee9536ec8 Mon Sep 17 00:00:00 2001 From: Sergey Konovalov Date: Sun, 11 Aug 2024 18:53:53 +0300 Subject: [PATCH] [bug] Set saveKey as postfix to fix vulnerability with path traversal to docId or other files; Fix bug 69505 --- DocService/sources/DocsCoServer.js | 2 +- DocService/sources/canvasservice.js | 26 +++++++++++++++----------- DocService/sources/converterservice.js | 1 - FileConverter/sources/converter.js | 18 +++++++++++++----- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/DocService/sources/DocsCoServer.js b/DocService/sources/DocsCoServer.js index 87554b7d7..4e0032774 100644 --- a/DocService/sources/DocsCoServer.js +++ b/DocService/sources/DocsCoServer.js @@ -864,7 +864,7 @@ async function checkForceSaveCache(ctx, convertInfo) { if (convertInfo) { res.hasCache = true; let cmd = new commonDefines.InputCommand(convertInfo, true); - const saveKey = cmd.getSaveKey(); + const saveKey = cmd.getDocId() + cmd.getSaveKey(); const outputPath = cmd.getOutputPath(); if (saveKey && outputPath) { const savePathDoc = saveKey + '/' + outputPath; diff --git a/DocService/sources/canvasservice.js b/DocService/sources/canvasservice.js index 0e069126b..789023b84 100644 --- a/DocService/sources/canvasservice.js +++ b/DocService/sources/canvasservice.js @@ -300,8 +300,10 @@ async function getOutputData(ctx, cmd, outputData, key, optConn, optAdditionalOu return status; } function* addRandomKeyTaskCmd(ctx, cmd) { - var task = yield* taskResult.addRandomKeyTask(ctx, cmd.getDocId()); - cmd.setSaveKey(task.key); + let docId = cmd.getDocId(); + let task = yield* taskResult.addRandomKeyTask(ctx, docId); + //set saveKey as postfix to fix vulnerability with path traversal to docId or other files + cmd.setSaveKey(task.key.substring(docId.length)); } function addPasswordToCmd(ctx, cmd, docPasswordStr) { let docPassword = sqlBase.DocumentPassword.prototype.getDocPassword(ctx, docPasswordStr); @@ -347,9 +349,9 @@ function* saveParts(ctx, cmd, filename) { } if (cmd.getUrl()) { result = true; - } else if (cmd.getData() && cmd.getData().length > 0) { + } else if (cmd.getData() && cmd.getData().length > 0 && cmd.getSaveKey()) { var buffer = cmd.getData(); - yield storage.putObject(ctx, cmd.getSaveKey() + '/' + filename, buffer, buffer.length); + yield storage.putObject(ctx, cmd.getDocId() + cmd.getSaveKey() + '/' + filename, buffer, buffer.length); //delete data to prevent serialize into json cmd.data = null; result = (SAVE_TYPE_COMPLETE_ALL === saveType || SAVE_TYPE_COMPLETE === saveType); @@ -378,7 +380,10 @@ async function getUpdateResponse(ctx, cmd) { var updateTask = new taskResult.TaskResultData(); updateTask.tenant = ctx.tenant; - updateTask.key = cmd.getSaveKey() ? cmd.getSaveKey() : cmd.getDocId(); + updateTask.key = cmd.getDocId(); + if (cmd.getSaveKey()) { + updateTask.key += cmd.getSaveKey(); + } var statusInfo = cmd.getStatusInfo(); if (constants.NO_ERROR === statusInfo) { updateTask.status = commonDefines.FileStatus.Ok; @@ -557,7 +562,6 @@ function* commandReopen(ctx, conn, cmd, outputData) { if (upsertRes.affectedRows > 0) { //add task cmd.setUrl(null);//url may expire - cmd.setSaveKey(cmd.getDocId()); cmd.setOutputFormat(docsCoServer.getOpenFormatByEditor(conn.editorType)); cmd.setEmbeddedFonts(false); if (isPassword) { @@ -933,7 +937,7 @@ const commandSfcCallback = co.wrap(function*(ctx, cmd, isSfcm, isEncrypted) { const userLastChangeIndex = cmd.getUserIndex() || cmd.getUserActionIndex(); let replyStr; if (constants.EDITOR_CHANGES !== statusInfo || isSfcm) { - var saveKey = cmd.getSaveKey(); + var saveKey = docId + cmd.getSaveKey(); var isError = constants.NO_ERROR != statusInfo; var isErrorCorrupted = constants.CONVERT_CORRUPTED == statusInfo; var savePathDoc = saveKey + '/' + cmd.getOutputPath(); @@ -1250,7 +1254,7 @@ function* processWopiPutFile(ctx, docId, wopiParams, savePathDoc, userLastChange function* commandSendMMCallback(ctx, cmd) { var docId = cmd.getDocId(); ctx.logger.debug('Start commandSendMMCallback'); - var saveKey = cmd.getSaveKey(); + var saveKey = docId + cmd.getSaveKey(); var statusInfo = cmd.getStatusInfo(); var outputSfc = new commonDefines.OutputSfcData(docId); if (constants.NO_ERROR == statusInfo) { @@ -1504,7 +1508,7 @@ exports.saveFile = function(req, res) { cmd.setStatusInfo(constants.NO_ERROR); yield* addRandomKeyTaskCmd(ctx, cmd); cmd.setOutputPath(constants.OUTPUT_NAME + pathModule.extname(cmd.getOutputPath())); - yield storage.putObject(ctx, cmd.getSaveKey() + '/' + cmd.getOutputPath(), req.body, req.body.length); + yield storage.putObject(ctx, docId + cmd.getSaveKey() + '/' + cmd.getOutputPath(), req.body, req.body.length); let replyStr = yield commandSfcCallback(ctx, cmd, false, true); if (replyStr) { utils.fillResponseSimple(res, replyStr, 'application/json'); @@ -1794,7 +1798,7 @@ async function processWopiSaveAs(ctx, cmd) { // info.wopiParams is null if it is not wopi if (info?.wopiParams) { const suggestedTargetType = `.${formatChecker.getStringFromFormat(cmd.getOutputFormat())}`; - const storageFilePath = `${cmd.getSaveKey()}/${cmd.getOutputPath()}`; + const storageFilePath = `${cmd.getDocId()}${cmd.getSaveKey()}/${cmd.getOutputPath()}`; const stream = await storage.createReadStream(ctx, storageFilePath); const { wopiSrc, access_token } = info.wopiParams.userAuth; await wopiClient.putRelativeFile(ctx, wopiSrc, access_token, null, stream.readStream, stream.contentLength, suggestedTargetType, false); @@ -1819,7 +1823,7 @@ exports.receiveTask = function(data, ack) { if ('open' === command || 'reopen' === command) { yield getOutputData(ctx, cmd, outputData, cmd.getDocId(), null, additionalOutput); } else if ('save' === command || 'savefromorigin' === command) { - let status = yield getOutputData(ctx, cmd, outputData, cmd.getSaveKey(), null, additionalOutput); + let status = yield getOutputData(ctx, cmd, outputData, cmd.getDocId() + cmd.getSaveKey(), null, additionalOutput); if (commonDefines.FileStatus.Ok === status && cmd.getIsSaveAs()) { yield processWopiSaveAs(ctx, cmd); //todo in case of wopi no need to send url. send it to avoid stubs in sdk diff --git a/DocService/sources/converterservice.js b/DocService/sources/converterservice.js index 85e85b8a9..8b1fa88b8 100644 --- a/DocService/sources/converterservice.js +++ b/DocService/sources/converterservice.js @@ -550,7 +550,6 @@ function convertTo(req, res) { let cmd = new commonDefines.InputCommand(); cmd.setCommand('conv'); cmd.setDocId(docId); - cmd.setSaveKey(docId); cmd.setFormat(filetype); cmd.setOutputFormat(outputFormat); cmd.setCodepage(commonDefines.c_oAscCodePageUtf8); diff --git a/FileConverter/sources/converter.js b/FileConverter/sources/converter.js index 41323acf4..e4f907b1f 100644 --- a/FileConverter/sources/converter.js +++ b/FileConverter/sources/converter.js @@ -96,7 +96,10 @@ let inputLimitsXmlCache; function TaskQueueDataConvert(ctx, task) { var cmd = task.getCmd(); - this.key = cmd.savekey ? cmd.savekey : cmd.id; + this.key = cmd.getDocId(); + if (cmd.getSaveKey()) { + this.key += cmd.getSaveKey(); + } this.fileFrom = null; this.fileTo = null; this.title = cmd.getTitle(); @@ -483,7 +486,10 @@ function* processDownloadFromStorage(ctx, dataConvert, cmd, task, tempDirs, auth if (task.getFromChanges()) { let changesDir = path.join(tempDirs.source, constants.CHANGES_NAME); fs.mkdirSync(changesDir); - let filesCount = yield* downloadFileFromStorage(ctx, cmd.getSaveKey(), changesDir); + let filesCount = 0; + if (cmd.getSaveKey()) { + filesCount = yield* downloadFileFromStorage(ctx, cmd.getDocId() + cmd.getSaveKey(), changesDir); + } if (filesCount > 0) { concatDir = changesDir; concatTemplate = "changes0"; @@ -495,7 +501,9 @@ function* processDownloadFromStorage(ctx, dataConvert, cmd, task, tempDirs, auth dataConvert.fileFrom = path.join(tempDirs.source, 'origin.' + cmd.getFormat()); } else { //overwrite some files from m_sKey (for example Editor.bin or changes) - yield* downloadFileFromStorage(ctx, cmd.getSaveKey(), tempDirs.source); + if (cmd.getSaveKey()) { + yield* downloadFileFromStorage(ctx, cmd.getDocId() + cmd.getSaveKey(), tempDirs.source); + } let format = cmd.getFormat() || 'bin'; dataConvert.fileFrom = path.join(tempDirs.source, 'Editor.' + format); concatDir = tempDirs.source; @@ -506,7 +514,7 @@ function* processDownloadFromStorage(ctx, dataConvert, cmd, task, tempDirs, auth //mail merge let mailMergeSend = cmd.getMailMergeSend(); if (mailMergeSend) { - yield* downloadFileFromStorage(ctx, mailMergeSend.getJsonKey(), tempDirs.source); + yield* downloadFileFromStorage(ctx, cmd.getDocId() + mailMergeSend.getJsonKey(), tempDirs.source); concatDir = tempDirs.source; } if (concatDir) { @@ -1078,7 +1086,7 @@ function* ExecuteTask(ctx, task) { } else { error = constants.CONVERT_PARAMS; } - } else if (cmd.getSaveKey()) { + } else if (cmd.getSaveKey() || task.getFromOrigin() || task.getFromSettings()) { yield* downloadFileFromStorage(ctx, cmd.getDocId(), tempDirs.source); ctx.logger.debug('downloadFileFromStorage complete'); if(clientStatsD) {