🛡️ : Security Enhancements (#1681)

* 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
This commit is contained in:
Danny Avila 2024-01-30 14:34:02 -05:00 committed by GitHub
parent 9fad1b2cae
commit 972402e029
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
23 changed files with 72 additions and 28 deletions

View file

@ -53,6 +53,7 @@ describe('Tool Handlers', () => {
username: 'fakeuser', username: 'fakeuser',
email: 'fakeuser@example.com', email: 'fakeuser@example.com',
emailVerified: false, emailVerified: false,
// file deepcode ignore NoHardcodedPasswords/test: fake value
password: 'fakepassword123', password: 'fakepassword123',
avatar: '', avatar: '',
provider: 'local', provider: 'local',

View file

@ -3,23 +3,24 @@ const { logger } = require('~/config');
//handle duplicates //handle duplicates
const handleDuplicateKeyError = (err, res) => { const handleDuplicateKeyError = (err, res) => {
logger.error('Duplicate key error:', err.keyValue); 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 code = 409;
const error = `An document with that ${field} already exists.`; res
res.status(code).send({ messages: error, fields: field }); .status(code)
.send({ messages: `An document with that ${field} already exists.`, fields: field });
}; };
//handle validation errors //handle validation errors
const handleValidationError = (err, res) => { const handleValidationError = (err, res) => {
logger.error('Validation error:', err.errors); logger.error('Validation error:', err.errors);
let errors = Object.values(err.errors).map((el) => el.message); 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; let code = 400;
if (errors.length > 1) { if (errors.length > 1) {
const formattedErrors = errors.join(' '); errors = errors.join(' ');
res.status(code).send({ messages: formattedErrors, fields: fields }); res.status(code).send({ messages: `${JSON.stringify(errors)}`, fields: fields });
} else { } else {
res.status(code).send({ messages: errors, fields: fields }); res.status(code).send({ messages: `${JSON.stringify(errors)}`, fields: fields });
} }
}; };

View file

@ -26,6 +26,7 @@ const startServer = async () => {
await indexSync(); await indexSync();
const app = express(); const app = express();
app.disable('x-powered-by');
await AppService(app); await AppService(app);
app.get('/health', (_req, res) => res.status(200).send('OK')); app.get('/health', (_req, res) => res.status(200).send('OK'));

View file

@ -1,7 +1,9 @@
const request = require('supertest'); const request = require('supertest');
const express = require('express'); const express = require('express');
const routes = require('../'); const routes = require('../');
// file deepcode ignore UseCsurfForExpress/test: test
const app = express(); const app = express();
app.disable('x-powered-by');
app.use('/api/config', routes.config); app.use('/api/config', routes.config);
afterEach(() => { afterEach(() => {

View file

@ -1,14 +1,20 @@
const express = require('express'); const express = require('express');
const router = express.Router();
const { getConvosByPage, deleteConvos } = require('~/models/Conversation'); const { getConvosByPage, deleteConvos } = require('~/models/Conversation');
const requireJwtAuth = require('~/server/middleware/requireJwtAuth'); const requireJwtAuth = require('~/server/middleware/requireJwtAuth');
const { getConvo, saveConvo } = require('~/models'); const { getConvo, saveConvo } = require('~/models');
const { logger } = require('~/config'); const { logger } = require('~/config');
const router = express.Router();
router.use(requireJwtAuth); router.use(requireJwtAuth);
router.get('/', async (req, res) => { 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)); 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); const convo = await getConvo(req.user.id, conversationId);
if (convo) { if (convo) {
res.status(200).send(convo); res.status(200).json(convo);
} else { } else {
res.status(404).end(); res.status(404).end();
} }
@ -39,10 +45,10 @@ router.post('/clear', async (req, res) => {
try { try {
const dbResponse = await deleteConvos(req.user.id, filter); const dbResponse = await deleteConvos(req.user.id, filter);
res.status(201).send(dbResponse); res.status(201).json(dbResponse);
} catch (error) { } catch (error) {
logger.error('Error clearing conversations', 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 { try {
const dbResponse = await saveConvo(req.user.id, update); const dbResponse = await saveConvo(req.user.id, update);
res.status(201).send(dbResponse); res.status(201).json(dbResponse);
} catch (error) { } catch (error) {
logger.error('Error updating conversation', error); logger.error('Error updating conversation', error);
res.status(500).send(error); res.status(500).send('Error updating conversation');
} }
}); });

View file

@ -1,4 +1,5 @@
const { z } = require('zod'); const { z } = require('zod');
const path = require('path');
const fs = require('fs').promises; const fs = require('fs').promises;
const express = require('express'); const express = require('express');
const upload = require('./multer'); const upload = require('./multer');
@ -39,7 +40,12 @@ router.post('/', upload.single('file'), async (req, res) => {
} catch (error) { } catch (error) {
logger.error('[/files/images] Error processing file:', error); logger.error('[/files/images] Error processing file:', error);
try { 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) { } catch (error) {
logger.error('[/files/images] Error deleting file:', error); logger.error('[/files/images] Error deleting file:', error);
} }

View file

@ -36,7 +36,7 @@ router.put('/:conversationId/:messageId', validateMessageReq, async (req, res) =
const { messageId, model } = req.params; const { messageId, model } = req.params;
const { text } = req.body; const { text } = req.body;
const tokenCount = await countTokens(text, model); 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 // DELETE

View file

@ -1,3 +1,5 @@
// file deepcode ignore NoRateLimitingForLogin: Rate limiting is handled by the `loginLimiter` middleware
const passport = require('passport'); const passport = require('passport');
const express = require('express'); const express = require('express');
const router = express.Router(); const router = express.Router();

View file

@ -5,27 +5,28 @@ const requireJwtAuth = require('~/server/middleware/requireJwtAuth');
const { logger } = require('~/config'); const { logger } = require('~/config');
const router = express.Router(); 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); 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 || {}; const update = req.body || {};
update.presetId = update?.presetId || crypto.randomUUID(); update.presetId = update?.presetId || crypto.randomUUID();
try { try {
const preset = await savePreset(req.user.id, update); const preset = await savePreset(req.user.id, update);
res.status(201).send(preset); res.status(201).json(preset);
} catch (error) { } catch (error) {
logger.error('[/presets] error saving preset', 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 = {}; let filter = {};
const { presetId } = req.body || {}; const { presetId } = req.body || {};
@ -37,10 +38,10 @@ router.post('/delete', requireJwtAuth, async (req, res) => {
try { try {
const deleteCount = await deletePresets(req.user.id, filter); const deleteCount = await deletePresets(req.user.id, filter);
res.status(201).send(deleteCount); res.status(201).json(deleteCount);
} catch (error) { } catch (error) {
logger.error('[/presets/delete] error deleting presets', 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');
} }
}); });

View file

@ -11,7 +11,7 @@ router.post('/', requireJwtAuth, async (req, res) => {
res.send({ count }); res.send({ count });
} catch (e) { } catch (e) {
logger.error('[/tokenizer] Error counting tokens', e); logger.error('[/tokenizer] Error counting tokens', e);
res.status(500).send(e.message); res.status(500).json('Error counting tokens');
} }
}); });

View file

@ -1,3 +1,5 @@
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets
const initializeClient = require('./initializeClient'); const initializeClient = require('./initializeClient');
const { GoogleClient } = require('~/app'); const { GoogleClient } = require('~/app');
const { checkUserKeyExpiry, getUserKey } = require('../../UserService'); const { checkUserKeyExpiry, getUserKey } = require('../../UserService');

View file

@ -93,7 +93,7 @@ async function saveFileFromURL({ userId, URL, fileName, basePath = 'images' }) {
} }
// Create a writable stream for the output path // 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); const writer = fs.createWriteStream(outputFilePath);
// Pipe the response data to the output file // Pipe the response data to the output file

View file

@ -1,3 +1,5 @@
// file deepcode ignore NoHardcodedPasswords: No hard-coded passwords in tests
const { loginSchema, registerSchema, errorsToString } = require('./validators'); const { loginSchema, registerSchema, errorsToString } = require('./validators');
describe('Zod Schemas', () => { describe('Zod Schemas', () => {

View file

@ -1,3 +1,5 @@
// file deepcode ignore HardcodedNonCryptoSecret: No hardcoded secrets
import React from 'react'; import React from 'react';
import type { TOpenAIMessage } from 'librechat-data-provider'; import type { TOpenAIMessage } from 'librechat-data-provider';
import { formatJSON, extractJson, isJson } from '~/utils/json'; import { formatJSON, extractJson, isJson } from '~/utils/json';

View file

@ -39,6 +39,7 @@ describe('PluginAuthForm', () => {
action: 'install', action: 'install',
auth: { auth: {
key: '1234567890', key: '1234567890',
// file deepcode ignore HardcodedNonCryptoSecret/test: test
secret: '1234567890', secret: '1234567890',
}, },
}); });

View file

@ -1,4 +1,6 @@
// Portuguese phrases // 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 { export default {
com_ui_examples: 'Exemplos', com_ui_examples: 'Exemplos',

View file

@ -1,4 +1,6 @@
// English phrases // 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 { export default {
com_ui_examples: 'Examples', com_ui_examples: 'Examples',

View file

@ -1,4 +1,6 @@
// Spanish phrases // 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 { export default {
com_ui_examples: 'Ejemplos', com_ui_examples: 'Ejemplos',

View file

@ -1,4 +1,6 @@
// French phrases // 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 { export default {
com_ui_examples: 'Exemples', com_ui_examples: 'Exemples',

View file

@ -1,4 +1,6 @@
// Indonesia phrases // 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 { export default {
com_ui_examples: 'Contoh', com_ui_examples: 'Contoh',

View file

@ -1,4 +1,6 @@
// Italian phrases // 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 { export default {
com_ui_examples: 'Esempi', com_ui_examples: 'Esempi',

View file

@ -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 { export default {
com_ui_examples: '例', com_ui_examples: '例',
@ -266,7 +268,8 @@ export default {
com_nav_welcome_message: 'How can I help you today?', com_nav_welcome_message: 'How can I help you today?',
com_nav_auto_scroll: 'チャットを開いたときに最新まで自動でスクロール', com_nav_auto_scroll: 'チャットを開いたときに最新まで自動でスクロール',
com_nav_modular_chat: '会話の途中でのエンドポイント切替を有効化', com_nav_modular_chat: '会話の途中でのエンドポイント切替を有効化',
com_nav_latex_parsing: 'メッセージ内の LaTeX の構文解析 (パフォーマンスに影響する可能性があります。)', com_nav_latex_parsing:
'メッセージ内の LaTeX の構文解析 (パフォーマンスに影響する可能性があります。)',
com_nav_profile_picture: 'プロフィール画像', com_nav_profile_picture: 'プロフィール画像',
com_nav_change_picture: '画像を変更', com_nav_change_picture: '画像を変更',
com_nav_plugin_store: 'プラグインストア', com_nav_plugin_store: 'プラグインストア',

View file

@ -1,4 +1,6 @@
// Chinese phrases // 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 { export default {
com_ui_examples: '示例', com_ui_examples: '示例',