[bug] Set saveKey as postfix to fix vulnerability with path traversal…

… to docId or other files; Fix bug 69505
ONLYOFFICE · Aug 11, 2024 · a9e4960 · a9e4960
1 parent 767e5b1
commit a9e4960
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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) {