Merge branch 'feat/duplicate-with-subordinate-page' into feat/gw-4274

CHANGES.md

@@ -1,6 +1,22 @@
 # CHANGES
 
-## v4.2.0-RC
+## v4.2.3-RC
+
+* 
+
+## v4.2.2
+
+* Fix: Consecutive save operations with built-in editor fail
+    * Introduced by v4.2.1
+
+## v4.2.1
+
+* Fix: Consecutive save operations with HackMD fail
+    * Introduced by v4.2.0
+* Fix: Switching theme to kibela fail
+    * Introduced by v4.2.0
+
+## v4.2.0
 
 ### BREAKING CHANGES
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "growi",
-  "version": "4.2.0-RC",
+  "version": "4.2.3-RC",
   "description": "Team collaboration software using markdown",
   "tags": [
     "wiki",

resource/locales/en_US/translation.json

@@ -164,7 +164,7 @@
     "page_not_exist_alert": "This page does not exist. Please create a new page."
   },
   "custom_navigation": {
-    "no_page_list": "There are no pages under <a href='{{path}}'><strong>{{ path }}</strong></a>."
+    "no_page_list": "There are no pages under this page."
   },
   "installer": {
     "setup": "Setup",

resource/locales/ja_JP/translation.json

@@ -167,7 +167,7 @@
     "page_not_exist_alert": "このページは存在しません。新たに作成する必要があります。"
   },
   "custom_navigation": {
-    "no_page_list": "<a href='{{path}}'><strong>{{ path }}</strong></a>の配下にはページが存在しません。"
+    "no_page_list": "このページの配下にはページが存在しません。"
   },
   "installer": {
     "setup": "セットアップ",

resource/locales/zh_CN/translation.json

@@ -165,7 +165,7 @@
     "page_not_exist_alert": "该页面不存在，请创建一个新页面"
   },
   "custom_navigation": {
-    "no_page_list": "There are no pages under <a href='{{path}}'><strong>{{ path }}</strong></a>."
+    "no_page_list": "There are no pages under this page."
   },
 	"installer": {
 		"setup": "安装",

src/client/js/components/Page/RevisionLoader.jsx

@@ -22,7 +22,7 @@ class RevisionLoader extends React.Component {
       markdown: '',
       isLoading: false,
       isLoaded: false,
-      error: null,
+      errors: null,
     };
 
     this.loadData = this.loadData.bind(this);
@@ -49,15 +49,15 @@ class RevisionLoader extends React.Component {
 
       this.setState({
         markdown: res.data.revision.body,
-        error: null,
+        errors: null,
       });
 
       if (this.props.onRevisionLoaded != null) {
         this.props.onRevisionLoaded(res.data.revision);
       }
     }
-    catch (error) {
-      this.setState({ error });
+    catch (errors) {
+      this.setState({ errors });
     }
     finally {
       this.setState({ isLoaded: true, isLoading: false });
@@ -94,8 +94,11 @@ class RevisionLoader extends React.Component {
 
     // ----- after load -----
     let markdown = this.state.markdown;
-    if (this.state.error != null) {
-      markdown = `<span class="text-muted"><em>${this.state.error}</em></span>`;
+    if (this.state.errors != null) {
+      const errorMessages = this.state.errors.map((error) => {
+        return `<span class="text-muted"><em>${error.message}</em></span>`;
+      });
+      markdown = errorMessages.join('');
     }
 
     return (

src/client/js/components/PageEditor.jsx

@@ -166,7 +166,7 @@ class PageEditor extends React.Component {
       // when if created newly
       if (res.pageCreated) {
         logger.info('Page is created', res.page._id);
-        pageContainer.updateStateAfterSave(res.page);
+        pageContainer.updateStateAfterSave(res.page, res.tags, res.revision);
         editorContainer.setState({ grant: res.page.grant });
       }
     }

src/client/js/components/PageEditorByHackmd.jsx

@@ -145,8 +145,10 @@ class PageEditorByHackmd extends React.Component {
       }
 
       this.props.pageContainer.setState({
+        isHackmdDraftUpdatingInRealtime: false,
         hasDraftOnHackmd: false,
         pageIdOnHackmd: res.pageIdOnHackmd,
+        remoteRevisionId: res.revisionIdHackmdSynced,
         revisionIdHackmdSynced: res.revisionIdHackmdSynced,
       });
     }

src/client/js/components/PageList.jsx

@@ -60,7 +60,7 @@ const PageList = (props) => {
     return (
       <div className="mt-2">
         {/* eslint-disable-next-line react/no-danger */}
-        <p dangerouslySetInnerHTML={{ __html: t('custom_navigation.no_page_list', { path }) }} />
+        <p>{t('custom_navigation.no_page_list')}</p>
       </div>
     );
   }

src/client/js/components/PageTimeline.jsx

@@ -62,12 +62,12 @@ class PageTimeline extends React.Component {
   render() {
     const { t } = this.props;
     const { pages } = this.state;
-    const { path } = this.props.pageContainer.state;
+
     if (pages == null || pages.length === 0) {
       return (
         <div className="mt-2">
           {/* eslint-disable-next-line react/no-danger */}
-          <p dangerouslySetInnerHTML={{ __html: t('custom_navigation.no_page_list', { path }) }} />
+          <p>{t('custom_navigation.no_page_list')}</p>
         </div>
       );
     }

src/client/js/services/NavigationContainer.js

@@ -115,6 +115,7 @@ export default class NavigationContainer extends Container {
     if (editorMode === 'edit') {
       $('body').addClass('on-edit');
       $('body').addClass('builtin-editor');
+      $('body').removeClass('hackmd');
       window.location.hash = '#edit';
     }
 

src/client/js/services/PageContainer.js

@@ -339,19 +339,20 @@ export default class PageContainer extends Container {
    * save success handler
    * @param {object} page Page instance
    * @param {Array[Tag]} tags Array of Tag
+   * @param {object} revision Revision instance
    */
-  updateStateAfterSave(page, tags) {
+  updateStateAfterSave(page, tags, revision) {
     const { editorMode } = this.navigationContainer.state;
 
     // update state of PageContainer
     const newState = {
       pageId: page._id,
-      revisionId: page.revision._id,
-      revisionCreatedAt: new Date(page.revision.createdAt).getTime() / 1000,
-      remoteRevisionId: page.revision._id,
+      revisionId: revision._id,
+      revisionCreatedAt: new Date(revision.createdAt).getTime() / 1000,
+      remoteRevisionId: revision._id,
       revisionIdHackmdSynced: page.revisionHackmdSynced,
       hasDraftOnHackmd: page.hasDraftOnHackmd,
-      markdown: page.revision.body,
+      markdown: revision.body,
       createdAt: page.createdAt,
       updatedAt: page.updatedAt,
     };
@@ -408,7 +409,7 @@ export default class PageContainer extends Container {
       res = await this.updatePage(pageId, revisionId, markdown, options);
     }
 
-    this.updateStateAfterSave(res.page, res.tags);
+    this.updateStateAfterSave(res.page, res.tags, res.revision);
     return res;
   }
 
@@ -488,7 +489,7 @@ export default class PageContainer extends Container {
     if (!res.ok) {
       throw new Error(res.error);
     }
-    return { page: res.page, tags: res.tags };
+    return res;
   }
 
   deletePage(isRecursively, isCompletely) {

src/client/styles/scss/theme/_apply-colors-dark.scss

@@ -47,12 +47,6 @@ textarea.form-control {
   // border: 1px solid darken($border, 30%);
 }
 
-.grw-slack-notification {
-  .form-control {
-    background: $bgcolor-global;
-  }
-}
-
 .form-control[disabled],
 .form-control[readonly] {
   color: lighten($color-global, 10%);
@@ -271,6 +265,43 @@ ul.pagination {
   .popover-body {
     color: inherit;
   }
+
+  &.bs-popover-top .arrow {
+    &::before {
+      border-top-color: $secondary;
+    }
+
+    &::after {
+      border-top-color: $bgcolor-global;
+    }
+  }
+  &.bs-popover-bottom .arrow {
+    &::before {
+      border-bottom-color: $secondary;
+    }
+
+    &::after {
+      border-bottom-color: $bgcolor-global;
+    }
+  }
+  &.bs-popover-right .arrow {
+    &::before {
+      border-right-color: $secondary;
+    }
+
+    &::after {
+      border-right-color: $bgcolor-global;
+    }
+  }
+  &.bs-popover-left .arrow {
+    &::before {
+      border-left-color: $secondary;
+    }
+
+    &::after {
+      border-left-color: $bgcolor-global;
+    }
+  }
 }
 
 /*
@@ -280,6 +311,10 @@ ul.pagination {
   background-color: transparent;
   $color-slack: #4b144c;
 
+  .form-control {
+    background: $bgcolor-global;
+  }
+
   .custom-control-label {
     &::before {
       background-color: $secondary;

src/client/styles/scss/theme/_apply-colors-light.scss

@@ -38,12 +38,6 @@ $border-color: $border-color-global;
   background-color: $bgcolor-global;
 }
 
-.grw-slack-notification {
-  .form-control {
-    background: white;
-  }
-}
-
 .form-control::placeholder {
   color: darken($bgcolor-global, 20%);
 }
@@ -191,6 +185,8 @@ $border-color: $border-color-global;
  * GROWI on-edit
  */
 .grw-editor-navbar-bottom {
+  background-color: $gray-50;
+
   #slack-mark-white {
     display: none;
   }
@@ -222,6 +218,10 @@ $border-color: $border-color-global;
   background-color: white;
   $color-slack: #4b144c;
 
+  .form-control {
+    background: white;
+  }
+
   .custom-control-label {
     &::before {
       background-color: $gray-200;

src/client/styles/scss/theme/_apply-colors.scss

@@ -433,10 +433,6 @@ body.on-edit {
       border-top-color: $border-color-theme;
     }
   }
-
-  .grw-editor-navbar-bottom {
-    background-color: $gray-50;
-  }
 }
 
 /*

src/server/crowi/index.js

@@ -134,7 +134,7 @@ Crowi.prototype.initForTest = async function() {
   // // slack depends on setUpSlacklNotification
   await Promise.all([
     this.setUpApp(),
-    // this.setUpXss(),
+    this.setUpXss(),
     // this.setUpSlacklNotification(),
     // this.setUpGrowiBridge(),
   ]);

src/server/models/serializers/page-serializer.js

@@ -7,11 +7,6 @@ function depopulate(page, attributeName) {
   }
 }
 
-function depopulateRevisions(page) {
-  depopulate(page, 'revision');
-  depopulate(page, 'revisionHackmdSynced');
-}
-
 function serializeInsecureUserAttributes(page) {
   if (page.lastUpdateUser != null && page.lastUpdateUser._id != null) {
     page.lastUpdateUser = serializeUserSecurely(page.lastUpdateUser);
@@ -25,7 +20,7 @@ function serializeInsecureUserAttributes(page) {
   return page;
 }
 
-function serializePageSecurely(page, shouldDepopulateRevisions = false) {
+function serializePageSecurely(page) {
   let serialized = page;
 
   // invoke toObject if page is a model instance
@@ -33,10 +28,9 @@ function serializePageSecurely(page, shouldDepopulateRevisions = false) {
     serialized = page.toObject();
   }
 
-  // optional depopulation
-  if (shouldDepopulateRevisions) {
-    depopulateRevisions(serialized);
-  }
+  // depopulate revision and revisionHackmdSynced
+  depopulate(serialized, 'revision');
+  depopulate(serialized, 'revisionHackmdSynced');
 
   serializeInsecureUserAttributes(serialized);
 

src/server/models/serializers/revision-serializer.js

@@ -0,0 +1,20 @@
+const { serializeUserSecurely } = require('./user-serializer');
+
+function serializeInsecureUserAttributes(revision) {
+  if (revision.author != null && revision.author._id != null) {
+    revision.author = serializeUserSecurely(revision.author);
+  }
+  return revision;
+}
+
+function serializeRevisionSecurely(revision) {
+  const serialized = revision;
+
+  serializeInsecureUserAttributes(serialized);
+
+  return serialized;
+}
+
+module.exports = {
+  serializeRevisionSecurely,
+};

src/server/models/vo/s2c-message.js

@@ -7,7 +7,7 @@ class S2cMessagePageUpdated {
 
 
   constructor(page, user) {
-    const serializedPage = serializePageSecurely(page, true);
+    const serializedPage = serializePageSecurely(page);
 
     const {
       _id, revision, revisionHackmdSynced, hasDraftOnHackmd,

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/6
+      // 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

@@ -4,6 +4,7 @@ const logger = loggerFactory('growi:routes:apiv3:export');
 const fs = require('fs');
 
 const express = require('express');
+const { param } = require('express-validator');
 
 const router = express.Router();
 
@@ -41,6 +42,7 @@ module.exports = (crowi) => {
   const accessTokenParser = require('../../middlewares/access-token-parser')(crowi);
   const loginRequired = require('../../middlewares/login-required')(crowi);
   const adminRequired = require('../../middlewares/admin-required')(crowi);
+  const apiV3FormValidator = require('../../middlewares/apiv3-form-validator')(crowi);
   const csrf = require('../../middlewares/csrf')(crowi);
 
   const { exportService, socketIoService } = crowi;
@@ -58,6 +60,14 @@ module.exports = (crowi) => {
     socketIoService.getAdminSocket().emit('admin:onTerminateForExport', data);
   });
 
+  const validator = {
+    deleteFile: [
+      // https://regex101.com/r/mD4eZs/6
+      // prevent from unexpecting attack doing delete file (path traversal attack)
+      param('fileName').not().matches(/(\.\.\/|\.\.\\)/),
+    ],
+  };
+
 
   /**
    * @swagger
@@ -150,7 +160,7 @@ module.exports = (crowi) => {
    *              schema:
    *                type: object
    */
-  router.delete('/:fileName', accessTokenParser, loginRequired, adminRequired, csrf, async(req, res) => {
+  router.delete('/:fileName', accessTokenParser, loginRequired, adminRequired, validator.deleteFile, apiV3FormValidator, csrf, async(req, res) => {
     // TODO: add express validator
     const { fileName } = req.params;
 

src/server/routes/apiv3/healthcheck.js

@@ -48,6 +48,31 @@ const ErrorV3 = require('../../models/vo/error-apiv3');
  */
 
 module.exports = (crowi) => {
+
+  async function checkMongo(errors, info) {
+    try {
+      const Config = crowi.models.Config;
+      await Config.findOne({});
+
+      info.mongo = 'OK';
+    }
+    catch (err) {
+      errors.push(new ErrorV3(`MongoDB is not connectable - ${err.message}`, 'healthcheck-mongodb-unhealthy', err.stack));
+    }
+  }
+
+  async function checkSearch(errors, info) {
+    const { searchService } = crowi;
+    if (searchService.isConfigured) {
+      try {
+        info.searchInfo = await searchService.getInfoForHealth();
+      }
+      catch (err) {
+        errors.push(new ErrorV3(`The Search Service is not connectable - ${err.message}`, 'healthcheck-search-unhealthy', err.stack));
+      }
+    }
+  }
+
   /**
    * @swagger
    *
@@ -58,14 +83,19 @@ module.exports = (crowi) => {
    *      summary: /healthcheck
    *      description: Check whether the server is healthy or not
    *      parameters:
-   *        - name: connectToMiddlewares
+   *        - name: checkServices
    *          in: query
-   *          description: Check MongoDB and SearchService (consider as healthy even if any of middleware is available or not)
+   *          description: The list of services to check health
    *          schema:
-   *            type: boolean
-   *        - name: checkMiddlewaresStrictly
+   *            type: array
+   *            items:
+   *              type: string
+   *              enum:
+   *                - mongo
+   *                - search
+   *        - name: strictly
    *          in: query
-   *          description: Check MongoDB and SearchService and responds 503 if either of these is unhealthy
+   *          description: Check services and responds 503 if either of these is unhealthy
    *          schema:
    *            type: boolean
    *      responses:
@@ -92,11 +122,22 @@ module.exports = (crowi) => {
    *                    $ref: '#/components/schemas/HealthcheckInfo'
    */
   router.get('/', helmet.noCache(), async(req, res) => {
-    const connectToMiddlewares = req.query.connectToMiddlewares != null;
-    const checkMiddlewaresStrictly = req.query.checkMiddlewaresStrictly != null;
+    let checkServices = req.query.checkServices || [];
+    let isStrictly = req.query.strictly != null;
 
-    // return 200 w/o connecting to MongoDB and SearchService
-    if (!connectToMiddlewares && !checkMiddlewaresStrictly) {
+    // for backward compatibility
+    if (req.query.connectToMiddlewares != null) {
+      logger.warn('The param \'connectToMiddlewares\' is deprecated. Use \'checkServices[]\' instead.');
+      checkServices = ['mongo', 'search'];
+    }
+    if (req.query.checkMiddlewaresStrictly != null) {
+      logger.warn('The param \'checkMiddlewaresStrictly\' is deprecated. Use \'checkServices[]\' and \'strictly\' instead.');
+      checkServices = ['mongo', 'search'];
+      isStrictly = true;
+    }
+
+    // return 200 w/o checking
+    if (checkServices.length === 0) {
       res.status(200).send({ status: 'OK' });
       return;
     }
@@ -105,30 +146,18 @@ module.exports = (crowi) => {
     const info = {};
 
     // connect to MongoDB
-    try {
-      const Config = crowi.models.Config;
-      await Config.findOne({});
-
-      info.mongo = 'OK';
-    }
-    catch (err) {
-      errors.push(new ErrorV3(`MongoDB is not connectable - ${err.message}`, 'healthcheck-mongodb-unhealthy', err.stack));
+    if (checkServices.includes('mongo')) {
+      await checkMongo(errors, info);
     }
 
     // connect to search service
-    const { searchService } = crowi;
-    if (searchService.isConfigured) {
-      try {
-        info.searchInfo = await searchService.getInfoForHealth();
-      }
-      catch (err) {
-        errors.push(new ErrorV3(`The Search Service is not connectable - ${err.message}`, 'healthcheck-search-unhealthy', err.stack));
-      }
+    if (checkServices.includes('search')) {
+      await checkSearch(errors, info);
     }
 
     if (errors.length > 0) {
       let httpStatus = 200;
-      if (checkMiddlewaresStrictly) {
+      if (isStrictly) {
         httpStatus = 503;
       }
 

src/server/routes/apiv3/mongo.js

@@ -14,6 +14,9 @@ const router = express.Router();
  */
 
 module.exports = (crowi) => {
+  const loginRequiredStrictly = require('../../middlewares/login-required')(crowi);
+  const adminRequired = require('../../middlewares/admin-required')(crowi);
+
   /**
    * @swagger
    *
@@ -35,7 +38,7 @@ module.exports = (crowi) => {
    *                    items:
    *                      type: string
    */
-  router.get('/collections', async(req, res) => {
+  router.get('/collections', loginRequiredStrictly, adminRequired, async(req, res) => {
     const listCollectionsResult = await mongoose.connection.db.listCollections().toArray();
     const collections = listCollectionsResult.map(collectionObj => collectionObj.name);
 

src/server/routes/attachment.js

@@ -3,6 +3,9 @@
 
 const logger = require('@alias/logger')('growi:routes:attachment');
 
+const { serializePageSecurely } = require('../models/serializers/page-serializer');
+const { serializeRevisionSecurely } = require('../models/serializers/revision-serializer');
+
 const ApiResponse = require('../util/apiResponse');
 
 /**
@@ -236,7 +239,7 @@ module.exports = function(crowi, app) {
     else {
       res.set({
         'Content-Type': attachment.fileFormat,
-        'Content-Security-Policy': "script-src 'unsafe-hashes'",
+        'Content-Security-Policy': "script-src 'unsafe-hashes'; object-src 'none'; require-trusted-types-for 'script'; default-src 'none';",
       });
     }
   }
@@ -466,7 +469,8 @@ module.exports = function(crowi, app) {
     }
 
     const result = {
-      page: page.toObject(),
+      page: serializePageSecurely(page),
+      revision: serializeRevisionSecurely(page.revision),
       attachment: attachment.toObject({ virtuals: true }),
       pageCreated,
     };

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/routes/page.js

@@ -1,4 +1,5 @@
 const { serializePageSecurely } = require('../models/serializers/page-serializer');
+const { serializeRevisionSecurely } = require('../models/serializers/revision-serializer');
 
 /**
  * @swagger
@@ -146,6 +147,17 @@ module.exports = function(crowi, app) {
   const interceptorManager = crowi.getInterceptorManager();
   const globalNotificationService = crowi.getGlobalNotificationService();
 
+  const XssOption = require('../../lib/service/xss/xssOption');
+  const Xss = require('../../lib/service/xss/index');
+  const initializedConfig = {
+    isEnabledXssPrevention: crowi.configManager.getConfig('markdown', 'markdown:xss:isEnabledPrevention'),
+    tagWhiteList: crowi.xssService.getTagWhiteList(),
+    attrWhiteList: crowi.xssService.getAttrWhiteList(),
+  };
+  const xssOption = new XssOption(initializedConfig);
+  const xss = new Xss(xssOption);
+
+
   const actions = {};
 
   function getPathFromRequest(req) {
@@ -229,6 +241,11 @@ module.exports = function(crowi, app) {
   }
 
   function addRenderVarsForPresentation(renderVars, page) {
+    // sanitize page.revision.body
+    if (crowi.configManager.getConfig('markdown', 'markdown:xss:isEnabledPrevention')) {
+      const preventXssRevision = xss.process(page.revision.body);
+      page.revision.body = preventXssRevision;
+    }
     renderVars.page = page;
     renderVars.revision = page.revision;
   }
@@ -733,7 +750,11 @@ module.exports = function(crowi, app) {
       savedTags = await PageTagRelation.listTagNamesByPage(createdPage.id);
     }
 
-    const result = { page: serializePageSecurely(createdPage), tags: savedTags };
+    const result = {
+      page: serializePageSecurely(createdPage),
+      revision: serializeRevisionSecurely(createdPage.revision),
+      tags: savedTags,
+    };
     res.json(ApiResponse.success(result));
 
     // update scopes for descendants
@@ -792,6 +813,8 @@ module.exports = function(crowi, app) {
    *                      $ref: '#/components/schemas/V1Response/properties/ok'
    *                    page:
    *                      $ref: '#/components/schemas/Page'
+   *                    revision:
+   *                      $ref: '#/components/schemas/Revision'
    *          403:
    *            $ref: '#/components/responses/403'
    *          500:
@@ -862,7 +885,11 @@ module.exports = function(crowi, app) {
       savedTags = await PageTagRelation.listTagNamesByPage(pageId);
     }
 
-    const result = { page: serializePageSecurely(page), tags: savedTags };
+    const result = {
+      page: serializePageSecurely(page),
+      revision: serializeRevisionSecurely(page.revision),
+      tags: savedTags,
+    };
     res.json(ApiResponse.success(result));
 
     // update scopes for descendants

src/server/routes/tag.js

@@ -82,7 +82,12 @@ module.exports = function(crowi, app) {
    * @apiParam {String} q keyword
    */
   api.search = async function(req, res) {
-    let tags = await Tag.find({ name: new RegExp(`^${req.query.q}`) }).select('-_id name');
+    // https://regex101.com/r/J1cN6O/1
+    // prevent from unexpecting attack doing regular expression on tag search (DoS attack)
+    // Search for regular expressions as normal characters
+    // e.g. user*$ -> user\*\$ (escape a regular expression)
+    const escapeRegExp = req.query.q.replace(/[\\^$/.*+?()[\]{}|]/g, '\\$&');
+    let tags = await Tag.find({ name: new RegExp(`^${escapeRegExp}`) }).select('_id name');
     tags = tags.map((tag) => { return tag.name });
     return res.json(ApiResponse.success({ tags }));
   };

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,6 +369,14 @@ class ImportService {
 
     unzipStream.on('entry', (entry) => {
       const fileName = entry.path;
+      // https://regex101.com/r/mD4eZs/6
+      // prevent from unexpecting attack doing unzip file (path traversal attack)
+      // FOR EXAMPLE
+      // ../../src/server/views/admin/markdown.html
+      if (fileName.match(/(\.\.\/|\.\.\\)/)) {
+        logger.error('File path is not appropriate.', fileName);
+        return;
+      }
 
       if (fileName === this.growiBridgeService.getMetaFileName()) {
         // skip meta.json