Merge branch 'master' into feat/3176-grid-edit-modal-for-master-merge

src/server/routes/admin.js

@@ -22,7 +22,7 @@ module.exports = function(crowi, app) {
   const MAX_PAGE_LIST = 50;
   const actions = {};
 
-  const { check } = require('express-validator');
+  const { check, param } = require('express-validator');
 
   const api = {};
 
@@ -316,13 +316,29 @@ module.exports = function(crowi, app) {
 
   // Export management
   actions.export = {};
+  actions.export.api = api;
+  api.validators.export = {};
+
   actions.export.index = (req, res) => {
     return res.render('admin/export');
   };
 
+  api.validators.export.download = function() {
+    const validator = [
+      // https://regex101.com/r/mD4eZs/4
+      // prevent from pass traversal attack
+      param('fileName').not().matches(/(\.\.\/|\.\.\\)/),
+    ];
+    return validator;
+  };
+
   actions.export.download = (req, res) => {
-    // TODO: add express validator
     const { fileName } = req.params;
+    const { validationResult } = require('express-validator');
+    const errors = validationResult(req);
+    if (!errors.isEmpty()) {
+      return res.status(422).json({ errors: `${fileName} is invalid. Do not use path like '../'.` });
+    }
 
     try {
       const zipFile = exportService.getFile(fileName);

src/server/routes/apiv3/export.js

@@ -62,9 +62,9 @@ module.exports = (crowi) => {
 
   const validator = {
     deleteFile: [
-      // https://regex101.com/r/mD4eZs/3
+      // https://regex101.com/r/mD4eZs/4
       // prevent from unexpecting attack doing delete file (path traversal attack)
-      param('fileName').not().matches(/(\.\.\/|\.\.\\)/g),
+      param('fileName').not().matches(/(\.\.\/|\.\.\\)/),
     ],
   };
 

src/server/routes/index.js

@@ -111,7 +111,7 @@ module.exports = function(crowi, app) {
 
   // export management for admin
   app.get('/admin/export'                       , loginRequiredStrictly , adminRequired ,admin.export.index);
-  app.get('/admin/export/:fileName'             , loginRequiredStrictly , adminRequired ,admin.export.download);
+  app.get('/admin/export/:fileName'             , loginRequiredStrictly , adminRequired ,admin.export.api.validators.export.download(), admin.export.download);
 
   app.get('/me'                       , loginRequiredStrictly , me.index);
   // external-accounts

src/server/service/file-uploader/aws.js

@@ -18,13 +18,9 @@ module.exports = function(crowi) {
     };
   }
 
-  function S3Factory(isUploadable) {
+  function S3Factory() {
     const awsConfig = getAwsConfig();
 
-    if (!isUploadable) {
-      throw new Error('AWS is not configured.');
-    }
-
     aws.config.update({
       accessKeyId: awsConfig.accessKeyId,
       secretAccessKey: awsConfig.secretAccessKey,
@@ -83,7 +79,11 @@ module.exports = function(crowi) {
   };
 
   lib.deleteFileByFilePath = async function(filePath) {
-    const s3 = S3Factory(this.getIsUploadable());
+    if (!this.getIsUploadable()) {
+      throw new Error('AWS is not configured.');
+    }
+
+    const s3 = S3Factory();
     const awsConfig = getAwsConfig();
 
     const params = {
@@ -102,9 +102,13 @@ module.exports = function(crowi) {
   };
 
   lib.uploadFile = function(fileStream, attachment) {
+    if (!this.getIsUploadable()) {
+      throw new Error('AWS is not configured.');
+    }
+
     logger.debug(`File uploading: fileName=${attachment.fileName}`);
 
-    const s3 = S3Factory(this.getIsUploadable());
+    const s3 = S3Factory();
     const awsConfig = getAwsConfig();
 
     const filePath = getFilePathOnStorage(attachment);
@@ -126,7 +130,11 @@ module.exports = function(crowi) {
    * @return {stream.Readable} readable stream
    */
   lib.findDeliveryFile = async function(attachment) {
-    const s3 = S3Factory(this.getIsUploadable());
+    if (!this.getIsReadable()) {
+      throw new Error('AWS is not configured.');
+    }
+
+    const s3 = S3Factory();
     const awsConfig = getAwsConfig();
     const filePath = getFilePathOnStorage(attachment);
 

src/server/service/file-uploader/gcs.js

@@ -15,10 +15,7 @@ module.exports = function(crowi) {
     return configManager.getConfig('crowi', 'gcs:bucket');
   }
 
-  function getGcsInstance(isUploadable) {
-    if (!isUploadable) {
-      throw new Error('GCS is not configured.');
-    }
+  function getGcsInstance() {
     if (_instance == null) {
       const keyFilename = configManager.getConfig('crowi', 'gcs:apiKeyJsonPath');
       // see https://googleapis.dev/nodejs/storage/latest/Storage.html
@@ -59,7 +56,11 @@ module.exports = function(crowi) {
   };
 
   lib.deleteFileByFilePath = async function(filePath) {
-    const gcs = getGcsInstance(this.getIsUploadable());
+    if (!this.getIsUploadable()) {
+      throw new Error('GCS is not configured.');
+    }
+
+    const gcs = getGcsInstance();
     const myBucket = gcs.bucket(getGcsBucket());
     const file = myBucket.file(filePath);
 
@@ -74,9 +75,13 @@ module.exports = function(crowi) {
   };
 
   lib.uploadFile = function(fileStream, attachment) {
+    if (!this.getIsUploadable()) {
+      throw new Error('GCS is not configured.');
+    }
+
     logger.debug(`File uploading: fileName=${attachment.fileName}`);
 
-    const gcs = getGcsInstance(this.getIsUploadable());
+    const gcs = getGcsInstance();
     const myBucket = gcs.bucket(getGcsBucket());
     const filePath = getFilePathOnStorage(attachment);
     const options = {
@@ -93,7 +98,11 @@ module.exports = function(crowi) {
    * @return {stream.Readable} readable stream
    */
   lib.findDeliveryFile = async function(attachment) {
-    const gcs = getGcsInstance(this.getIsUploadable());
+    if (!this.getIsReadable()) {
+      throw new Error('GCS is not configured.');
+    }
+
+    const gcs = getGcsInstance();
     const myBucket = gcs.bucket(getGcsBucket());
     const filePath = getFilePathOnStorage(attachment);
     const file = myBucket.file(filePath);

src/server/service/file-uploader/uploader.js

@@ -12,6 +12,11 @@ class Uploader {
     return !this.configManager.getConfig('crowi', 'app:fileUploadDisabled') && this.isValidUploadSettings();
   }
 
+  // File reading is possible even if uploading is disabled
+  getIsReadable() {
+    return this.isValidUploadSettings();
+  }
+
   isValidUploadSettings() {
     throw new Error('Implement this');
   }

src/server/service/import.js

@@ -369,11 +369,11 @@ class ImportService {
 
     unzipStream.on('entry', (entry) => {
       const fileName = entry.path;
-      // https://regex101.com/r/mD4eZs/3
+      // https://regex101.com/r/mD4eZs/4
       // prevent from unexpecting attack doing unzip file (path traversal attack)
       // FOR EXAMPLE
       // ../../src/server/views/admin/markdown.html
-      if (fileName.match(/(\.\.\/|\.\.\\)/g)) {
+      if (fileName.match(/(\.\.\/|\.\.\\)/)) {
         logger.error('File path is not appropriate.', fileName);
         return;
       }