From 972402e02945aba054ae4d6a56d08c7e684cc3bc Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 30 Jan 2024 14:34:02 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20:=20Security=20Enhancem?= =?UTF-8?q?ents=20(#1681)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: sanitize HTTP params and do not send whole error objects backs * fix: prevent path traversal * fix: send custom error message for tokenizer route * chore: handle info exposure vector * chore(oauth): skip check due to false positive as oauth routes are rate-limited * chore(app): disable `x-powered-by` * chore: disable false positives or flagging of hardcoded secrets when they are fake values * chore: add path traversal safety check --- .../clients/tools/util/handleTools.test.js | 1 + api/server/controllers/ErrorController.js | 15 +++++++------- api/server/index.js | 1 + api/server/routes/__tests__/config.spec.js | 2 ++ api/server/routes/convos.js | 20 ++++++++++++------- api/server/routes/files/images.js | 8 +++++++- api/server/routes/messages.js | 2 +- api/server/routes/oauth.js | 2 ++ api/server/routes/presets.js | 17 ++++++++-------- api/server/routes/tokenizer.js | 2 +- .../Endpoints/google/initializeClient.spec.js | 2 ++ api/server/services/Files/Local/crud.js | 2 +- api/strategies/validators.spec.js | 2 ++ .../src/components/Messages/Content/Error.tsx | 2 ++ .../Store/__tests__/PluginAuthForm.spec.tsx | 1 + client/src/localization/languages/Br.tsx | 2 ++ client/src/localization/languages/Eng.tsx | 2 ++ client/src/localization/languages/Es.tsx | 2 ++ client/src/localization/languages/Fr.tsx | 2 ++ client/src/localization/languages/Id.tsx | 2 ++ client/src/localization/languages/It.tsx | 2 ++ client/src/localization/languages/Jp.tsx | 7 +++++-- client/src/localization/languages/Zh.tsx | 2 ++ 23 files changed, 72 insertions(+), 28 deletions(-) diff --git a/api/app/clients/tools/util/handleTools.test.js b/api/app/clients/tools/util/handleTools.test.js index 40d8bc6129e..3586495515a 100644 --- a/api/app/clients/tools/util/handleTools.test.js +++ b/api/app/clients/tools/util/handleTools.test.js @@ -53,6 +53,7 @@ describe('Tool Handlers', () => { username: 'fakeuser', email: 'fakeuser@example.com', emailVerified: false, + // file deepcode ignore NoHardcodedPasswords/test: fake value password: 'fakepassword123', avatar: '', provider: 'local', diff --git a/api/server/controllers/ErrorController.js b/api/server/controllers/ErrorController.js index 1308527b8cd..234cb90fb37 100644 --- a/api/server/controllers/ErrorController.js +++ b/api/server/controllers/ErrorController.js @@ -3,23 +3,24 @@ const { logger } = require('~/config'); //handle duplicates const handleDuplicateKeyError = (err, res) => { logger.error('Duplicate key error:', err.keyValue); - const field = Object.keys(err.keyValue); + const field = `${JSON.stringify(Object.keys(err.keyValue))}`; const code = 409; - const error = `An document with that ${field} already exists.`; - res.status(code).send({ messages: error, fields: field }); + res + .status(code) + .send({ messages: `An document with that ${field} already exists.`, fields: field }); }; //handle validation errors const handleValidationError = (err, res) => { logger.error('Validation error:', err.errors); let errors = Object.values(err.errors).map((el) => el.message); - let fields = Object.values(err.errors).map((el) => el.path); + let fields = `${JSON.stringify(Object.values(err.errors).map((el) => el.path))}`; let code = 400; if (errors.length > 1) { - const formattedErrors = errors.join(' '); - res.status(code).send({ messages: formattedErrors, fields: fields }); + errors = errors.join(' '); + res.status(code).send({ messages: `${JSON.stringify(errors)}`, fields: fields }); } else { - res.status(code).send({ messages: errors, fields: fields }); + res.status(code).send({ messages: `${JSON.stringify(errors)}`, fields: fields }); } }; diff --git a/api/server/index.js b/api/server/index.js index 9d7d3f8afdc..c08415e8fd4 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -26,6 +26,7 @@ const startServer = async () => { await indexSync(); const app = express(); + app.disable('x-powered-by'); await AppService(app); app.get('/health', (_req, res) => res.status(200).send('OK')); diff --git a/api/server/routes/__tests__/config.spec.js b/api/server/routes/__tests__/config.spec.js index 4833b83d105..4032d23b5c3 100644 --- a/api/server/routes/__tests__/config.spec.js +++ b/api/server/routes/__tests__/config.spec.js @@ -1,7 +1,9 @@ const request = require('supertest'); const express = require('express'); const routes = require('../'); +// file deepcode ignore UseCsurfForExpress/test: test const app = express(); +app.disable('x-powered-by'); app.use('/api/config', routes.config); afterEach(() => { diff --git a/api/server/routes/convos.js b/api/server/routes/convos.js index 4395df0fee1..a410b6fd584 100644 --- a/api/server/routes/convos.js +++ b/api/server/routes/convos.js @@ -1,14 +1,20 @@ const express = require('express'); -const router = express.Router(); const { getConvosByPage, deleteConvos } = require('~/models/Conversation'); const requireJwtAuth = require('~/server/middleware/requireJwtAuth'); const { getConvo, saveConvo } = require('~/models'); const { logger } = require('~/config'); +const router = express.Router(); router.use(requireJwtAuth); router.get('/', async (req, res) => { - const pageNumber = req.query.pageNumber || 1; + let pageNumber = req.query.pageNumber || 1; + pageNumber = parseInt(pageNumber, 10); + + if (isNaN(pageNumber) || pageNumber < 1) { + return res.status(400).json({ error: 'Invalid page number' }); + } + res.status(200).send(await getConvosByPage(req.user.id, pageNumber)); }); @@ -17,7 +23,7 @@ router.get('/:conversationId', async (req, res) => { const convo = await getConvo(req.user.id, conversationId); if (convo) { - res.status(200).send(convo); + res.status(200).json(convo); } else { res.status(404).end(); } @@ -39,10 +45,10 @@ router.post('/clear', async (req, res) => { try { const dbResponse = await deleteConvos(req.user.id, filter); - res.status(201).send(dbResponse); + res.status(201).json(dbResponse); } catch (error) { logger.error('Error clearing conversations', error); - res.status(500).send(error); + res.status(500).send('Error clearing conversations'); } }); @@ -51,10 +57,10 @@ router.post('/update', async (req, res) => { try { const dbResponse = await saveConvo(req.user.id, update); - res.status(201).send(dbResponse); + res.status(201).json(dbResponse); } catch (error) { logger.error('Error updating conversation', error); - res.status(500).send(error); + res.status(500).send('Error updating conversation'); } }); diff --git a/api/server/routes/files/images.js b/api/server/routes/files/images.js index 30d3c3cac60..d1016f98f75 100644 --- a/api/server/routes/files/images.js +++ b/api/server/routes/files/images.js @@ -1,4 +1,5 @@ const { z } = require('zod'); +const path = require('path'); const fs = require('fs').promises; const express = require('express'); const upload = require('./multer'); @@ -39,7 +40,12 @@ router.post('/', upload.single('file'), async (req, res) => { } catch (error) { logger.error('[/files/images] Error processing file:', error); try { - await fs.unlink(file.path); + const filepath = path.join( + req.app.locals.paths.imageOutput, + req.user.id, + path.basename(file.filename), + ); + await fs.unlink(filepath); } catch (error) { logger.error('[/files/images] Error deleting file:', error); } diff --git a/api/server/routes/messages.js b/api/server/routes/messages.js index 1e2faafe7bf..d53dacae495 100644 --- a/api/server/routes/messages.js +++ b/api/server/routes/messages.js @@ -36,7 +36,7 @@ router.put('/:conversationId/:messageId', validateMessageReq, async (req, res) = const { messageId, model } = req.params; const { text } = req.body; const tokenCount = await countTokens(text, model); - res.status(201).send(await updateMessage({ messageId, text, tokenCount })); + res.status(201).json(await updateMessage({ messageId, text, tokenCount })); }); // DELETE diff --git a/api/server/routes/oauth.js b/api/server/routes/oauth.js index 816fc7200f3..e85d83d8883 100644 --- a/api/server/routes/oauth.js +++ b/api/server/routes/oauth.js @@ -1,3 +1,5 @@ +// file deepcode ignore NoRateLimitingForLogin: Rate limiting is handled by the `loginLimiter` middleware + const passport = require('passport'); const express = require('express'); const router = express.Router(); diff --git a/api/server/routes/presets.js b/api/server/routes/presets.js index 76aaed698cd..19214a3a7d1 100644 --- a/api/server/routes/presets.js +++ b/api/server/routes/presets.js @@ -5,27 +5,28 @@ const requireJwtAuth = require('~/server/middleware/requireJwtAuth'); const { logger } = require('~/config'); const router = express.Router(); +router.use(requireJwtAuth); -router.get('/', requireJwtAuth, async (req, res) => { +router.get('/', async (req, res) => { const presets = (await getPresets(req.user.id)).map((preset) => preset); - res.status(200).send(presets); + res.status(200).json(presets); }); -router.post('/', requireJwtAuth, async (req, res) => { +router.post('/', async (req, res) => { const update = req.body || {}; update.presetId = update?.presetId || crypto.randomUUID(); try { const preset = await savePreset(req.user.id, update); - res.status(201).send(preset); + res.status(201).json(preset); } catch (error) { logger.error('[/presets] error saving preset', error); - res.status(500).send(error); + res.status(500).send('There was an error when saving the preset'); } }); -router.post('/delete', requireJwtAuth, async (req, res) => { +router.post('/delete', async (req, res) => { let filter = {}; const { presetId } = req.body || {}; @@ -37,10 +38,10 @@ router.post('/delete', requireJwtAuth, async (req, res) => { try { const deleteCount = await deletePresets(req.user.id, filter); - res.status(201).send(deleteCount); + res.status(201).json(deleteCount); } catch (error) { logger.error('[/presets/delete] error deleting presets', error); - res.status(500).send(error); + res.status(500).send('There was an error deleting the presets'); } }); diff --git a/api/server/routes/tokenizer.js b/api/server/routes/tokenizer.js index 581f82bf2ad..e12a86bde16 100644 --- a/api/server/routes/tokenizer.js +++ b/api/server/routes/tokenizer.js @@ -11,7 +11,7 @@ router.post('/', requireJwtAuth, async (req, res) => { res.send({ count }); } catch (e) { logger.error('[/tokenizer] Error counting tokens', e); - res.status(500).send(e.message); + res.status(500).json('Error counting tokens'); } }); diff --git a/api/server/services/Endpoints/google/initializeClient.spec.js b/api/server/services/Endpoints/google/initializeClient.spec.js index 8587c71e2d8..e39e51b8571 100644 --- a/api/server/services/Endpoints/google/initializeClient.spec.js +++ b/api/server/services/Endpoints/google/initializeClient.spec.js @@ -1,3 +1,5 @@ +// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets + const initializeClient = require('./initializeClient'); const { GoogleClient } = require('~/app'); const { checkUserKeyExpiry, getUserKey } = require('../../UserService'); diff --git a/api/server/services/Files/Local/crud.js b/api/server/services/Files/Local/crud.js index d81c063031a..cd8bdbc5d8a 100644 --- a/api/server/services/Files/Local/crud.js +++ b/api/server/services/Files/Local/crud.js @@ -93,7 +93,7 @@ async function saveFileFromURL({ userId, URL, fileName, basePath = 'images' }) { } // Create a writable stream for the output path - const outputFilePath = path.join(outputPath, fileName); + const outputFilePath = path.join(outputPath, path.basename(fileName)); const writer = fs.createWriteStream(outputFilePath); // Pipe the response data to the output file diff --git a/api/strategies/validators.spec.js b/api/strategies/validators.spec.js index bd4e2192fbb..365818b65b8 100644 --- a/api/strategies/validators.spec.js +++ b/api/strategies/validators.spec.js @@ -1,3 +1,5 @@ +// file deepcode ignore NoHardcodedPasswords: No hard-coded passwords in tests + const { loginSchema, registerSchema, errorsToString } = require('./validators'); describe('Zod Schemas', () => { diff --git a/client/src/components/Messages/Content/Error.tsx b/client/src/components/Messages/Content/Error.tsx index bf2f2b9a241..9505d4eba69 100644 --- a/client/src/components/Messages/Content/Error.tsx +++ b/client/src/components/Messages/Content/Error.tsx @@ -1,3 +1,5 @@ +// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets + import React from 'react'; import type { TOpenAIMessage } from 'librechat-data-provider'; import { formatJSON, extractJson, isJson } from '~/utils/json'; diff --git a/client/src/components/Plugins/Store/__tests__/PluginAuthForm.spec.tsx b/client/src/components/Plugins/Store/__tests__/PluginAuthForm.spec.tsx index a895295caa3..7d9676d2485 100644 --- a/client/src/components/Plugins/Store/__tests__/PluginAuthForm.spec.tsx +++ b/client/src/components/Plugins/Store/__tests__/PluginAuthForm.spec.tsx @@ -39,6 +39,7 @@ describe('PluginAuthForm', () => { action: 'install', auth: { key: '1234567890', + // file deepcode ignore HardcodedNonCryptoSecret/test: test secret: '1234567890', }, }); diff --git a/client/src/localization/languages/Br.tsx b/client/src/localization/languages/Br.tsx index 197a078f96b..260e33145a5 100644 --- a/client/src/localization/languages/Br.tsx +++ b/client/src/localization/languages/Br.tsx @@ -1,4 +1,6 @@ // Portuguese phrases +// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file +// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file export default { com_ui_examples: 'Exemplos', diff --git a/client/src/localization/languages/Eng.tsx b/client/src/localization/languages/Eng.tsx index 189b57b3513..c49c0d0dfd6 100644 --- a/client/src/localization/languages/Eng.tsx +++ b/client/src/localization/languages/Eng.tsx @@ -1,4 +1,6 @@ // English phrases +// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file +// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file export default { com_ui_examples: 'Examples', diff --git a/client/src/localization/languages/Es.tsx b/client/src/localization/languages/Es.tsx index 5934c2874d8..f4525b78182 100644 --- a/client/src/localization/languages/Es.tsx +++ b/client/src/localization/languages/Es.tsx @@ -1,4 +1,6 @@ // Spanish phrases +// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file +// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file export default { com_ui_examples: 'Ejemplos', diff --git a/client/src/localization/languages/Fr.tsx b/client/src/localization/languages/Fr.tsx index 100cff5987b..3e986c0c861 100644 --- a/client/src/localization/languages/Fr.tsx +++ b/client/src/localization/languages/Fr.tsx @@ -1,4 +1,6 @@ // French phrases +// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file +// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file export default { com_ui_examples: 'Exemples', diff --git a/client/src/localization/languages/Id.tsx b/client/src/localization/languages/Id.tsx index cc8d7cfe5ab..f4a7d1cf19e 100644 --- a/client/src/localization/languages/Id.tsx +++ b/client/src/localization/languages/Id.tsx @@ -1,4 +1,6 @@ // Indonesia phrases +// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file +// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file export default { com_ui_examples: 'Contoh', diff --git a/client/src/localization/languages/It.tsx b/client/src/localization/languages/It.tsx index 2fe949fee97..094cbb3e898 100644 --- a/client/src/localization/languages/It.tsx +++ b/client/src/localization/languages/It.tsx @@ -1,4 +1,6 @@ // Italian phrases +// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file +// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file export default { com_ui_examples: 'Esempi', diff --git a/client/src/localization/languages/Jp.tsx b/client/src/localization/languages/Jp.tsx index a48675688a6..a1795df37d0 100644 --- a/client/src/localization/languages/Jp.tsx +++ b/client/src/localization/languages/Jp.tsx @@ -1,4 +1,6 @@ -// English phrases +// Japanese phrases +// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file +// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file export default { com_ui_examples: '例', @@ -266,7 +268,8 @@ export default { com_nav_welcome_message: 'How can I help you today?', com_nav_auto_scroll: 'チャットを開いたときに最新まで自動でスクロール', com_nav_modular_chat: '会話の途中でのエンドポイント切替を有効化', - com_nav_latex_parsing: 'メッセージ内の LaTeX の構文解析 (パフォーマンスに影響する可能性があります。)', + com_nav_latex_parsing: + 'メッセージ内の LaTeX の構文解析 (パフォーマンスに影響する可能性があります。)', com_nav_profile_picture: 'プロフィール画像', com_nav_change_picture: '画像を変更', com_nav_plugin_store: 'プラグインストア', diff --git a/client/src/localization/languages/Zh.tsx b/client/src/localization/languages/Zh.tsx index b4542b0b769..4706348edcc 100644 --- a/client/src/localization/languages/Zh.tsx +++ b/client/src/localization/languages/Zh.tsx @@ -1,4 +1,6 @@ // Chinese phrases +// file deepcode ignore NoHardcodedPasswords: No hardcoded values present in this file +// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets present in this file export default { com_ui_examples: '示例',