From 4638cd713dcdd007cd7f49b9a95fa62ac7823e7c Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 15 Nov 2022 14:41:55 +0100 Subject: Don't inject untrusted input Even if it's already checked in middlewares It's better to have safe modals too --- server/middlewares/pagination.ts | 5 +++-- server/middlewares/validators/abuse.ts | 3 ++- server/middlewares/validators/redundancy.ts | 3 ++- server/middlewares/validators/shared/abuses.ts | 3 ++- server/middlewares/validators/shared/accounts.ts | 5 +++-- server/middlewares/validators/shared/users.ts | 3 ++- server/middlewares/validators/shared/video-comments.ts | 7 ++++--- server/middlewares/validators/shared/video-ownerships.ts | 3 ++- server/middlewares/validators/users.ts | 3 ++- server/middlewares/validators/videos/video-imports.ts | 3 ++- server/middlewares/validators/videos/video-playlists.ts | 3 ++- 11 files changed, 26 insertions(+), 15 deletions(-) (limited to 'server/middlewares') diff --git a/server/middlewares/pagination.ts b/server/middlewares/pagination.ts index 9812af9e4..17e43f743 100644 --- a/server/middlewares/pagination.ts +++ b/server/middlewares/pagination.ts @@ -1,12 +1,13 @@ import express from 'express' +import { forceNumber } from '@shared/core-utils' import { PAGINATION } from '../initializers/constants' function setDefaultPagination (req: express.Request, res: express.Response, next: express.NextFunction) { if (!req.query.start) req.query.start = 0 - else req.query.start = parseInt(req.query.start, 10) + else req.query.start = forceNumber(req.query.start) if (!req.query.count) req.query.count = PAGINATION.GLOBAL.COUNT.DEFAULT - else req.query.count = parseInt(req.query.count, 10) + else req.query.count = forceNumber(req.query.count) return next() } diff --git a/server/middlewares/validators/abuse.ts b/server/middlewares/validators/abuse.ts index 9b94008ce..70bae1775 100644 --- a/server/middlewares/validators/abuse.ts +++ b/server/middlewares/validators/abuse.ts @@ -18,6 +18,7 @@ import { AbuseMessageModel } from '@server/models/abuse/abuse-message' import { AbuseCreate, UserRight } from '@shared/models' import { HttpStatusCode } from '../../../shared/models/http/http-error-codes' import { areValidationErrors, doesAbuseExist, doesAccountIdExist, doesCommentIdExist, doesVideoExist } from './shared' +import { forceNumber } from '@shared/core-utils' const abuseReportValidator = [ body('account.id') @@ -216,7 +217,7 @@ const deleteAbuseMessageValidator = [ const user = res.locals.oauth.token.user const abuse = res.locals.abuse - const messageId = parseInt(req.params.messageId + '', 10) + const messageId = forceNumber(req.params.messageId) const abuseMessage = await AbuseMessageModel.loadByIdAndAbuseId(messageId, abuse.id) if (!abuseMessage) { diff --git a/server/middlewares/validators/redundancy.ts b/server/middlewares/validators/redundancy.ts index 79460f63c..c80f9b728 100644 --- a/server/middlewares/validators/redundancy.ts +++ b/server/middlewares/validators/redundancy.ts @@ -1,6 +1,7 @@ import express from 'express' import { body, param, query } from 'express-validator' import { isVideoRedundancyTarget } from '@server/helpers/custom-validators/video-redundancies' +import { forceNumber } from '@shared/core-utils' import { HttpStatusCode } from '../../../shared/models/http/http-error-codes' import { exists, @@ -171,7 +172,7 @@ const removeVideoRedundancyValidator = [ async (req: express.Request, res: express.Response, next: express.NextFunction) => { if (areValidationErrors(req, res)) return - const redundancy = await VideoRedundancyModel.loadByIdWithVideo(parseInt(req.params.redundancyId, 10)) + const redundancy = await VideoRedundancyModel.loadByIdWithVideo(forceNumber(req.params.redundancyId)) if (!redundancy) { return res.fail({ status: HttpStatusCode.NOT_FOUND_404, diff --git a/server/middlewares/validators/shared/abuses.ts b/server/middlewares/validators/shared/abuses.ts index 2b8d86ba5..2c988f9ec 100644 --- a/server/middlewares/validators/shared/abuses.ts +++ b/server/middlewares/validators/shared/abuses.ts @@ -1,9 +1,10 @@ import { Response } from 'express' import { AbuseModel } from '@server/models/abuse/abuse' import { HttpStatusCode } from '@shared/models' +import { forceNumber } from '@shared/core-utils' async function doesAbuseExist (abuseId: number | string, res: Response) { - const abuse = await AbuseModel.loadByIdWithReporter(parseInt(abuseId + '', 10)) + const abuse = await AbuseModel.loadByIdWithReporter(forceNumber(abuseId)) if (!abuse) { res.fail({ diff --git a/server/middlewares/validators/shared/accounts.ts b/server/middlewares/validators/shared/accounts.ts index fe4f83aa0..72b0e235e 100644 --- a/server/middlewares/validators/shared/accounts.ts +++ b/server/middlewares/validators/shared/accounts.ts @@ -2,10 +2,11 @@ import { Response } from 'express' import { AccountModel } from '@server/models/account/account' import { UserModel } from '@server/models/user/user' import { MAccountDefault } from '@server/types/models' +import { forceNumber } from '@shared/core-utils' import { HttpStatusCode } from '@shared/models' function doesAccountIdExist (id: number | string, res: Response, sendNotFound = true) { - const promise = AccountModel.load(parseInt(id + '', 10)) + const promise = AccountModel.load(forceNumber(id)) return doesAccountExist(promise, res, sendNotFound) } @@ -40,7 +41,7 @@ async function doesAccountExist (p: Promise, res: Response, sen } async function doesUserFeedTokenCorrespond (id: number, token: string, res: Response) { - const user = await UserModel.loadByIdWithChannels(parseInt(id + '', 10)) + const user = await UserModel.loadByIdWithChannels(forceNumber(id)) if (token !== user.feedToken) { res.fail({ diff --git a/server/middlewares/validators/shared/users.ts b/server/middlewares/validators/shared/users.ts index fbaa7db0e..b8f1436d3 100644 --- a/server/middlewares/validators/shared/users.ts +++ b/server/middlewares/validators/shared/users.ts @@ -2,10 +2,11 @@ import express from 'express' import { ActorModel } from '@server/models/actor/actor' import { UserModel } from '@server/models/user/user' import { MUserDefault } from '@server/types/models' +import { forceNumber } from '@shared/core-utils' import { HttpStatusCode } from '@shared/models' function checkUserIdExist (idArg: number | string, res: express.Response, withStats = false) { - const id = parseInt(idArg + '', 10) + const id = forceNumber(idArg) return checkUserExist(() => UserModel.loadByIdWithChannels(id, withStats), res) } diff --git a/server/middlewares/validators/shared/video-comments.ts b/server/middlewares/validators/shared/video-comments.ts index 8d1a16294..0961b3ec9 100644 --- a/server/middlewares/validators/shared/video-comments.ts +++ b/server/middlewares/validators/shared/video-comments.ts @@ -1,10 +1,11 @@ import express from 'express' import { VideoCommentModel } from '@server/models/video/video-comment' import { MVideoId } from '@server/types/models' +import { forceNumber } from '@shared/core-utils' import { HttpStatusCode, ServerErrorCode } from '@shared/models' async function doesVideoCommentThreadExist (idArg: number | string, video: MVideoId, res: express.Response) { - const id = parseInt(idArg + '', 10) + const id = forceNumber(idArg) const videoComment = await VideoCommentModel.loadById(id) if (!videoComment) { @@ -33,7 +34,7 @@ async function doesVideoCommentThreadExist (idArg: number | string, video: MVide } async function doesVideoCommentExist (idArg: number | string, video: MVideoId, res: express.Response) { - const id = parseInt(idArg + '', 10) + const id = forceNumber(idArg) const videoComment = await VideoCommentModel.loadByIdAndPopulateVideoAndAccountAndReply(id) if (!videoComment) { @@ -57,7 +58,7 @@ async function doesVideoCommentExist (idArg: number | string, video: MVideoId, r } async function doesCommentIdExist (idArg: number | string, res: express.Response) { - const id = parseInt(idArg + '', 10) + const id = forceNumber(idArg) const videoComment = await VideoCommentModel.loadByIdAndPopulateVideoAndAccountAndReply(id) if (!videoComment) { diff --git a/server/middlewares/validators/shared/video-ownerships.ts b/server/middlewares/validators/shared/video-ownerships.ts index 680613cda..33ac9c8b6 100644 --- a/server/middlewares/validators/shared/video-ownerships.ts +++ b/server/middlewares/validators/shared/video-ownerships.ts @@ -1,9 +1,10 @@ import express from 'express' import { VideoChangeOwnershipModel } from '@server/models/video/video-change-ownership' +import { forceNumber } from '@shared/core-utils' import { HttpStatusCode } from '@shared/models' async function doesChangeVideoOwnershipExist (idArg: number | string, res: express.Response) { - const id = parseInt(idArg + '', 10) + const id = forceNumber(idArg) const videoChangeOwnership = await VideoChangeOwnershipModel.load(id) if (!videoChangeOwnership) { diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts index 055af3b64..50327b6ae 100644 --- a/server/middlewares/validators/users.ts +++ b/server/middlewares/validators/users.ts @@ -1,6 +1,7 @@ import express from 'express' import { body, param, query } from 'express-validator' import { Hooks } from '@server/lib/plugins/hooks' +import { forceNumber } from '@shared/core-utils' import { HttpStatusCode, UserRegister, UserRight, UserRole } from '@shared/models' import { exists, isBooleanValid, isIdValid, toBooleanOrNull, toIntOrNull } from '../../helpers/custom-validators/misc' import { isThemeNameValid } from '../../helpers/custom-validators/plugins' @@ -515,7 +516,7 @@ const usersCheckCurrentPasswordFactory = (targetUserIdGetter: (req: express.Requ const user = res.locals.oauth.token.User const isAdminOrModerator = user.role === UserRole.ADMINISTRATOR || user.role === UserRole.MODERATOR - const targetUserId = parseInt(targetUserIdGetter(req) + '') + const targetUserId = forceNumber(targetUserIdGetter(req)) // Admin/moderator action on another user, skip the password check if (isAdminOrModerator && targetUserId !== user.id) { diff --git a/server/middlewares/validators/videos/video-imports.ts b/server/middlewares/validators/videos/video-imports.ts index f295b1885..72442aeb6 100644 --- a/server/middlewares/validators/videos/video-imports.ts +++ b/server/middlewares/validators/videos/video-imports.ts @@ -4,6 +4,7 @@ import { isResolvingToUnicastOnly } from '@server/helpers/dns' import { isPreImportVideoAccepted } from '@server/lib/moderation' import { Hooks } from '@server/lib/plugins/hooks' import { MUserAccountId, MVideoImport } from '@server/types/models' +import { forceNumber } from '@shared/core-utils' import { HttpStatusCode, UserRight, VideoImportState } from '@shared/models' import { VideoImportCreate } from '@shared/models/videos/import/video-import-create.model' import { isIdValid, toIntOrNull } from '../../../helpers/custom-validators/misc' @@ -130,7 +131,7 @@ const videoImportCancelValidator = [ async (req: express.Request, res: express.Response, next: express.NextFunction) => { if (areValidationErrors(req, res)) return - if (!await doesVideoImportExist(parseInt(req.params.id), res)) return + if (!await doesVideoImportExist(forceNumber(req.params.id), res)) return if (!checkUserCanManageImport(res.locals.oauth.token.user, res.locals.videoImport, res)) return if (res.locals.videoImport.state !== VideoImportState.PENDING) { diff --git a/server/middlewares/validators/videos/video-playlists.ts b/server/middlewares/validators/videos/video-playlists.ts index 6d4b8a6f1..e4b7e5c56 100644 --- a/server/middlewares/validators/videos/video-playlists.ts +++ b/server/middlewares/validators/videos/video-playlists.ts @@ -2,6 +2,7 @@ import express from 'express' import { body, param, query, ValidationChain } from 'express-validator' import { ExpressPromiseHandler } from '@server/types/express-handler' import { MUserAccountId } from '@server/types/models' +import { forceNumber } from '@shared/core-utils' import { HttpStatusCode, UserRight, @@ -258,7 +259,7 @@ const videoPlaylistElementAPGetValidator = [ async (req: express.Request, res: express.Response, next: express.NextFunction) => { if (areValidationErrors(req, res)) return - const playlistElementId = parseInt(req.params.playlistElementId + '', 10) + const playlistElementId = forceNumber(req.params.playlistElementId) const playlistId = req.params.playlistId const videoPlaylistElement = await VideoPlaylistElementModel.loadByPlaylistAndElementIdForAP(playlistId, playlistElementId) -- cgit v1.2.3