From 888190a6be4c103b6eec3e5c0f97f678eda87900 Mon Sep 17 00:00:00 2001 From: mikiher Date: Wed, 15 Oct 2025 18:28:15 +0300 Subject: [PATCH] Fix codeQL failures --- server/controllers/SearchController.js | 25 +++++++++++++------------ server/finders/BookFinder.js | 7 +++++-- server/utils/index.js | 19 +++++++++++++++++++ 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/server/controllers/SearchController.js b/server/controllers/SearchController.js index 72f602d2a..57538d2c0 100644 --- a/server/controllers/SearchController.js +++ b/server/controllers/SearchController.js @@ -4,7 +4,7 @@ const BookFinder = require('../finders/BookFinder') const PodcastFinder = require('../finders/PodcastFinder') const AuthorFinder = require('../finders/AuthorFinder') const Database = require('../Database') -const { isValidASIN } = require('../utils') +const { isValidASIN, getQueryParamAsString } = require('../utils') // Provider name mappings for display purposes const providerMap = { @@ -139,9 +139,10 @@ class SearchController { * @param {Response} res */ async findBooks(req, res) { - const provider = req.query.provider || 'google' - const title = req.query.title || '' - const author = req.query.author || '' + // Safely extract query parameters, rejecting arrays to prevent type confusion + const provider = getQueryParamAsString(req.query.provider, 'google') + const title = getQueryParamAsString(req.query.title, '') + const author = getQueryParamAsString(req.query.author, '') // Validate string parameters const validation = SearchController.validateStringParams({ provider, title, author }, 'findBooks') @@ -164,9 +165,9 @@ class SearchController { async findCovers(req, res) { const query = req.query const podcast = query.podcast === '1' || query.podcast === 1 - const title = query.title || '' - const author = query.author || '' - const provider = query.provider || 'google' + const title = getQueryParamAsString(query.title, '') + const author = getQueryParamAsString(query.author, '') + const provider = getQueryParamAsString(query.provider, 'google') // Validate required title const titleValidation = SearchController.validateRequiredString(title, 'title', 'findCovers') @@ -190,8 +191,8 @@ class SearchController { * @param {Response} res */ async findPodcasts(req, res) { - const term = req.query.term - const country = req.query.country || 'us' + const term = getQueryParamAsString(req.query.term) + const country = getQueryParamAsString(req.query.country, 'us') // Validate required term const termValidation = SearchController.validateRequiredString(term, 'term', 'findPodcasts') @@ -212,7 +213,7 @@ class SearchController { * @param {Response} res */ async findAuthor(req, res) { - const query = req.query.q + const query = getQueryParamAsString(req.query.q) // Validate query parameter const validation = SearchController.validateRequiredString(query, 'query', 'findAuthor') @@ -229,8 +230,8 @@ class SearchController { * @param {Response} res */ async findChapters(req, res) { - const asin = req.query.asin - const region = (req.query.region || 'us').toLowerCase() + const asin = getQueryParamAsString(req.query.asin) + const region = getQueryParamAsString(req.query.region, 'us').toLowerCase() // Validate ASIN parameter const asinValidation = SearchController.validateRequiredString(asin, 'asin', 'findChapters') diff --git a/server/finders/BookFinder.js b/server/finders/BookFinder.js index a6a6b07e6..6dc90c44f 100644 --- a/server/finders/BookFinder.js +++ b/server/finders/BookFinder.js @@ -402,7 +402,8 @@ class BookFinder { let authorCandidates = new BookFinder.AuthorCandidates(cleanAuthor, this.audnexus) // Remove underscores and parentheses with their contents, and replace with a separator - const cleanTitle = title.replace(/\[.*?\]|\(.*?\)|{.*?}|_/g, ' - ') + // Use negated character classes to prevent ReDoS vulnerability + const cleanTitle = title.replace(/\[[^\]]*\]|\([^)]*\)|{[^}]*}|_/g, ' - ') // Split title into hypen-separated parts const titleParts = cleanTitle.split(/ - | -|- /) for (const titlePart of titleParts) authorCandidates.add(titlePart) @@ -668,7 +669,9 @@ function cleanTitleForCompares(title, keepSubtitle = false) { let stripped = keepSubtitle ? title : stripSubtitle(title) // Remove text in paranthesis (i.e. "Ender's Game (Ender's Saga)" becomes "Ender's Game") - let cleaned = stripped.replace(/ *\([^)]*\) */g, '') + // Use a safe two-pass approach to prevent ReDoS vulnerability + let cleaned = stripped.replace(/\([^)]*\)/g, '') // Remove parenthetical content + cleaned = cleaned.replace(/\s+/g, ' ').trim() // Clean up any resulting multiple spaces // Remove single quotes (i.e. "Ender's Game" becomes "Enders Game") cleaned = cleaned.replace(/'/g, '') diff --git a/server/utils/index.js b/server/utils/index.js index 369620276..4421fbade 100644 --- a/server/utils/index.js +++ b/server/utils/index.js @@ -277,3 +277,22 @@ module.exports.timestampToSeconds = (timestamp) => { } return null } + +/** + * Safely extracts a query parameter as a string, rejecting arrays to prevent type confusion + * Express query parameters can be arrays if the same parameter appears multiple times + * @example ?author=Smith => "Smith" + * @example ?author=Smith&author=Jones => null (array detected) + * + * @param {any} value - Query parameter value + * @param {string} defaultValue - Default value if undefined/null + * @returns {string|null} String value or null if invalid (array) + */ +module.exports.getQueryParamAsString = (value, defaultValue = '') => { + // Explicitly reject arrays to prevent type confusion + if (Array.isArray(value)) { + return null + } + // Return default for undefined/null, otherwise return the value + return value == null ? defaultValue : value +}