Browse Source

Merge pull request #10264 from weseek/fix/170423-codeql-problems-3

fix: CodeQL problems
Shun Miyazawa 7 months ago
parent
commit
ba20d99686

+ 1 - 1
apps/app/src/server/routes/apiv3/attachment.js

@@ -356,7 +356,7 @@ module.exports = (crowi) => {
       }
 
       try {
-        const page = await Page.findById(pageId);
+        const page = await Page.findOne({ _id: { $eq: pageId } });
 
         // check the user is accessible
         const isAccessible = await Page.isAccessiblePageByViewer(page.id, req.user);

+ 42 - 22
apps/app/src/server/routes/apiv3/export.js

@@ -1,7 +1,12 @@
+import fs from 'fs';
+
+import { SCOPE } from '@growi/core/dist/interfaces';
+import express from 'express';
+import { param, body } from 'express-validator';
+import mongoose from 'mongoose';
 import sanitize from 'sanitize-filename';
 
 import { SupportedAction } from '~/interfaces/activity';
-import { SCOPE } from '@growi/core/dist/interfaces';
 import { accessTokenParser } from '~/server/middlewares/access-token-parser';
 import { exportService } from '~/server/service/export';
 import loggerFactory from '~/utils/logger';
@@ -11,11 +16,6 @@ import { apiV3FormValidator } from '../../middlewares/apiv3-form-validator';
 
 
 const logger = loggerFactory('growi:routes:apiv3:export');
-const fs = require('fs');
-
-const express = require('express');
-const { param } = require('express-validator');
-
 const router = express.Router();
 
 /**
@@ -145,6 +145,25 @@ module.exports = (crowi) => {
   });
 
   const validator = {
+    generateZipFile: [
+      body('collections')
+        .isArray()
+        .withMessage('"collections" must be an array')
+        .bail()
+
+        .notEmpty()
+        .withMessage('"collections" array cannot be empty')
+        .bail()
+
+        .custom(async(value) => {
+          // Check if all the collections in the request body exist in the database
+          const listCollectionsResult = await mongoose.connection.db.listCollections().toArray();
+          const allCollectionNames = listCollectionsResult.map(collectionObj => collectionObj.name);
+          if (!value.every(v => allCollectionNames.includes(v))) {
+            throw new Error('Invalid collections');
+          }
+        }),
+    ],
     deleteFile: [
       // https://regex101.com/r/mD4eZs/6
       // prevent from unexpecting attack doing delete file (path traversal attack)
@@ -214,27 +233,28 @@ module.exports = (crowi) => {
    *                    type: boolean
    *                    description: whether the request is succeeded
    */
-  router.post('/', accessTokenParser([SCOPE.WRITE.ADMIN.EXPORT_DATA], { acceptLegacy: true }), loginRequired, adminRequired, addActivity, async(req, res) => {
+  router.post('/', accessTokenParser([SCOPE.WRITE.ADMIN.EXPORT_DATA], { acceptLegacy: true }), loginRequired, adminRequired,
+    validator.generateZipFile, apiV3FormValidator, addActivity, async(req, res) => {
     // TODO: add express validator
-    try {
-      const { collections } = req.body;
+      try {
+        const { collections } = req.body;
 
-      exportService.export(collections);
+        exportService.export(collections);
 
-      const parameters = { action: SupportedAction.ACTION_ADMIN_ARCHIVE_DATA_CREATE };
-      activityEvent.emit('update', res.locals.activity._id, parameters);
+        const parameters = { action: SupportedAction.ACTION_ADMIN_ARCHIVE_DATA_CREATE };
+        activityEvent.emit('update', res.locals.activity._id, parameters);
 
-      // TODO: use res.apiv3
-      return res.status(200).json({
-        ok: true,
-      });
-    }
-    catch (err) {
+        // TODO: use res.apiv3
+        return res.status(200).json({
+          ok: true,
+        });
+      }
+      catch (err) {
       // TODO: use ApiV3Error
-      logger.error(err);
-      return res.status(500).send({ status: 'ERROR' });
-    }
-  });
+        logger.error(err);
+        return res.status(500).send({ status: 'ERROR' });
+      }
+    });
 
   /**
    * @swagger

+ 1 - 1
apps/app/src/server/routes/apiv3/page/update-page.ts

@@ -145,7 +145,7 @@ export const updatePageHandlersFactory: UpdatePageHandlersFactory = (crowi) => {
       const sanitizeRevisionId = revisionId == null ? undefined : generalXssFilter.process(revisionId);
 
       // check page existence
-      const isExist = await Page.count({ _id: pageId }) > 0;
+      const isExist = await Page.count({ _id: { $eq: pageId } }) > 0;
       if (!isExist) {
         return res.apiv3Err(new ErrorV3(`Page('${pageId}' is not found or forbidden`, 'notfound_or_forbidden'), 400);
       }

+ 2 - 0
packages/remark-lsx/src/client/components/Lsx.tsx

@@ -66,6 +66,8 @@ const LsxSubstance = React.memo(
             <span className="material-symbols-outlined me-1">warning</span>{' '}
             {lsxContext.toString()}
           </summary>
+          {/* Since error messages may contain user-input strings, use JSX embedding as shown below */}
+          {/* https://legacy.reactjs.org/docs/introducing-jsx.html#jsx-prevents-injection-attacks */}
           <small className="ms-3 text-muted">{errorMessage}</small>
         </details>
       );

+ 2 - 2
packages/remark-lsx/src/client/stores/lsx/lsx.ts

@@ -77,9 +77,9 @@ export const useSWRxLsx = (
         return res.data;
       } catch (err) {
         if (axios.isAxiosError(err)) {
-          throw new Error(err.response?.data.message);
+          throw new Error(err.response?.data);
         }
-        throw err;
+        throw new Error(err);
       }
     },