From 7b54a81cccf6b4c12269e9d6897d608b1a99537a Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 6 Jan 2022 11:16:35 +0100 Subject: Prevent video import on non unicast ips --- .../middlewares/validators/videos/video-imports.ts | 18 ++++++++++++++ server/tests/api/check-params/video-imports.ts | 28 ++++++++++++++++++++++ 2 files changed, 46 insertions(+) (limited to 'server') diff --git a/server/middlewares/validators/videos/video-imports.ts b/server/middlewares/validators/videos/video-imports.ts index 640139c73..e4b54283f 100644 --- a/server/middlewares/validators/videos/video-imports.ts +++ b/server/middlewares/validators/videos/video-imports.ts @@ -13,6 +13,7 @@ import { CONFIG } from '../../../initializers/config' import { CONSTRAINTS_FIELDS } from '../../../initializers/constants' import { areValidationErrors, doesVideoChannelOfAccountExist } from '../shared' import { getCommonVideoEditAttributes } from './videos' +import { isValid as isIPValid, parse as parseIP } from 'ipaddr.js' const videoImportAddValidator = getCommonVideoEditAttributes().concat([ body('channelId') @@ -71,6 +72,23 @@ const videoImportAddValidator = getCommonVideoEditAttributes().concat([ return res.fail({ message: 'Should have a magnetUri or a targetUrl or a torrent file.' }) } + if (req.body.targetUrl) { + const hostname = new URL(req.body.targetUrl).hostname + + if (isIPValid(hostname)) { + const parsed = parseIP(hostname) + + if (parsed.range() !== 'unicast') { + cleanUpReqFiles(req) + + return res.fail({ + status: HttpStatusCode.FORBIDDEN_403, + message: 'Cannot use non unicast IP as targetUrl.' + }) + } + } + } + if (!await isImportAccepted(req, res)) return cleanUpReqFiles(req) return next() diff --git a/server/tests/api/check-params/video-imports.ts b/server/tests/api/check-params/video-imports.ts index d6d745488..6c31daa9b 100644 --- a/server/tests/api/check-params/video-imports.ts +++ b/server/tests/api/check-params/video-imports.ts @@ -108,6 +108,34 @@ describe('Test video imports API validator', function () { await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) }) + it('Should fail with localhost', async function () { + const fields = { ...baseCorrectParams, targetUrl: 'http://localhost:8000' } + + await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) + }) + + it('Should fail with a private IP target urls', async function () { + const targetUrls = [ + 'http://127.0.0.1:8000', + 'http://127.0.0.1', + 'http://127.0.0.1/hello', + 'https://192.168.1.42', + 'http://192.168.1.42' + ] + + for (const targetUrl of targetUrls) { + const fields = { ...baseCorrectParams, targetUrl } + + await makePostBodyRequest({ + url: server.url, + path, + token: server.accessToken, + fields, + expectedStatus: HttpStatusCode.FORBIDDEN_403 + }) + } + }) + it('Should fail with a long name', async function () { const fields = { ...baseCorrectParams, name: 'super'.repeat(65) } -- cgit v1.2.3 From 795212f7acc690c88c86d0fab8772f6564d59cb8 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 6 Jan 2022 13:27:29 +0100 Subject: Prevent caption listing of private videos --- server/controllers/api/videos/captions.ts | 2 +- server/controllers/api/videos/files.ts | 4 ++- server/middlewares/validators/shared/videos.ts | 33 ++++++++++++++++++++-- .../validators/videos/video-captions.ts | 22 +++++++++++++-- server/middlewares/validators/videos/videos.ts | 19 ++++--------- server/tests/api/check-params/video-captions.ts | 28 +++++++++++++++++- 6 files changed, 86 insertions(+), 22 deletions(-) (limited to 'server') diff --git a/server/controllers/api/videos/captions.ts b/server/controllers/api/videos/captions.ts index 2d2213327..aa7259ee9 100644 --- a/server/controllers/api/videos/captions.ts +++ b/server/controllers/api/videos/captions.ts @@ -48,7 +48,7 @@ export { // --------------------------------------------------------------------------- async function listVideoCaptions (req: express.Request, res: express.Response) { - const data = await VideoCaptionModel.listVideoCaptions(res.locals.videoId.id) + const data = await VideoCaptionModel.listVideoCaptions(res.locals.onlyVideo.id) return res.json(getFormattedObjects(data, data.length)) } diff --git a/server/controllers/api/videos/files.ts b/server/controllers/api/videos/files.ts index a8b32411d..0fbda280e 100644 --- a/server/controllers/api/videos/files.ts +++ b/server/controllers/api/videos/files.ts @@ -10,13 +10,15 @@ import { ensureUserHasRight, videoFileMetadataGetValidator, videoFilesDeleteHLSValidator, - videoFilesDeleteWebTorrentValidator + videoFilesDeleteWebTorrentValidator, + videosGetValidator } from '../../../middlewares' const lTags = loggerTagsFactory('api', 'video') const filesRouter = express.Router() filesRouter.get('/:id/metadata/:videoFileId', + asyncMiddleware(videosGetValidator), asyncMiddleware(videoFileMetadataGetValidator), asyncMiddleware(getVideoFileMetadata) ) diff --git a/server/middlewares/validators/shared/videos.ts b/server/middlewares/validators/shared/videos.ts index 71b81654f..fc978b63a 100644 --- a/server/middlewares/validators/shared/videos.ts +++ b/server/middlewares/validators/shared/videos.ts @@ -1,16 +1,20 @@ -import { Response } from 'express' +import { Request, Response } from 'express' import { loadVideo, VideoLoadType } from '@server/lib/model-loaders' +import { authenticatePromiseIfNeeded } from '@server/middlewares/auth' +import { VideoModel } from '@server/models/video/video' import { VideoChannelModel } from '@server/models/video/video-channel' import { VideoFileModel } from '@server/models/video/video-file' import { MUser, MUserAccountId, + MVideo, MVideoAccountLight, MVideoFormattableDetails, MVideoFullLight, MVideoId, MVideoImmutable, - MVideoThumbnail + MVideoThumbnail, + MVideoWithRights } from '@server/types/models' import { HttpStatusCode, UserRight } from '@shared/models' @@ -89,6 +93,27 @@ async function doesVideoChannelOfAccountExist (channelId: number, user: MUserAcc return true } +async function checkCanSeeVideoIfPrivate (req: Request, res: Response, video: MVideo, authenticateInQuery = false) { + if (!video.requiresAuth()) return true + + const videoWithRights = await VideoModel.loadAndPopulateAccountAndServerAndTags(video.id) + + return checkCanSeePrivateVideo(req, res, videoWithRights, authenticateInQuery) +} + +async function checkCanSeePrivateVideo (req: Request, res: Response, video: MVideoWithRights, authenticateInQuery = false) { + await authenticatePromiseIfNeeded(req, res, authenticateInQuery) + + const user = res.locals.oauth ? res.locals.oauth.token.User : null + + // Only the owner or a user that have blocklist rights can see the video + if (!user || !user.canGetVideo(video)) { + return false + } + + return true +} + function checkUserCanManageVideo (user: MUser, video: MVideoAccountLight, right: UserRight, res: Response, onlyOwned = true) { // Retrieve the user who did the request if (onlyOwned && video.isOwned() === false) { @@ -120,5 +145,7 @@ export { doesVideoChannelOfAccountExist, doesVideoExist, doesVideoFileOfVideoExist, - checkUserCanManageVideo + checkUserCanManageVideo, + checkCanSeeVideoIfPrivate, + checkCanSeePrivateVideo } diff --git a/server/middlewares/validators/videos/video-captions.ts b/server/middlewares/validators/videos/video-captions.ts index 38321ccf9..4fc4c8ec5 100644 --- a/server/middlewares/validators/videos/video-captions.ts +++ b/server/middlewares/validators/videos/video-captions.ts @@ -1,11 +1,18 @@ import express from 'express' import { body, param } from 'express-validator' -import { UserRight } from '../../../../shared' +import { HttpStatusCode, UserRight } from '../../../../shared' import { isVideoCaptionFile, isVideoCaptionLanguageValid } from '../../../helpers/custom-validators/video-captions' import { cleanUpReqFiles } from '../../../helpers/express-utils' import { logger } from '../../../helpers/logger' import { CONSTRAINTS_FIELDS, MIMETYPES } from '../../../initializers/constants' -import { areValidationErrors, checkUserCanManageVideo, doesVideoCaptionExist, doesVideoExist, isValidVideoIdParam } from '../shared' +import { + areValidationErrors, + checkCanSeeVideoIfPrivate, + checkUserCanManageVideo, + doesVideoCaptionExist, + doesVideoExist, + isValidVideoIdParam +} from '../shared' const addVideoCaptionValidator = [ isValidVideoIdParam('videoId'), @@ -64,7 +71,16 @@ const listVideoCaptionsValidator = [ logger.debug('Checking listVideoCaptions parameters', { parameters: req.params }) if (areValidationErrors(req, res)) return - if (!await doesVideoExist(req.params.videoId, res, 'id')) return + if (!await doesVideoExist(req.params.videoId, res, 'only-video')) return + + const video = res.locals.onlyVideo + + if (!await checkCanSeeVideoIfPrivate(req, res, video)) { + return res.fail({ + status: HttpStatusCode.FORBIDDEN_403, + message: 'Cannot list captions of private/internal/blocklisted video' + }) + } return next() } diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index 3ebdbc33d..782f495e8 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -51,9 +51,9 @@ import { CONSTRAINTS_FIELDS, OVERVIEWS } from '../../../initializers/constants' import { isLocalVideoAccepted } from '../../../lib/moderation' import { Hooks } from '../../../lib/plugins/hooks' import { VideoModel } from '../../../models/video/video' -import { authenticatePromiseIfNeeded } from '../../auth' import { areValidationErrors, + checkCanSeePrivateVideo, checkUserCanManageVideo, doesVideoChannelOfAccountExist, doesVideoExist, @@ -317,19 +317,12 @@ const videosCustomGetValidator = ( // Video private or blacklisted if (video.requiresAuth()) { - await authenticatePromiseIfNeeded(req, res, authenticateInQuery) + if (await checkCanSeePrivateVideo(req, res, video, authenticateInQuery)) return next() - const user = res.locals.oauth ? res.locals.oauth.token.User : null - - // Only the owner or a user that have blocklist rights can see the video - if (!user || !user.canGetVideo(video)) { - return res.fail({ - status: HttpStatusCode.FORBIDDEN_403, - message: 'Cannot get this private/internal or blocklisted video' - }) - } - - return next() + return res.fail({ + status: HttpStatusCode.FORBIDDEN_403, + message: 'Cannot get this private/internal or blocklisted video' + }) } // Video is public, anyone can access it diff --git a/server/tests/api/check-params/video-captions.ts b/server/tests/api/check-params/video-captions.ts index 90f429314..84c6c1355 100644 --- a/server/tests/api/check-params/video-captions.ts +++ b/server/tests/api/check-params/video-captions.ts @@ -11,7 +11,7 @@ import { PeerTubeServer, setAccessTokensToServers } from '@shared/extra-utils' -import { HttpStatusCode, VideoCreateResult } from '@shared/models' +import { HttpStatusCode, VideoCreateResult, VideoPrivacy } from '@shared/models' describe('Test video captions API validator', function () { const path = '/api/v1/videos/' @@ -19,6 +19,7 @@ describe('Test video captions API validator', function () { let server: PeerTubeServer let userAccessToken: string let video: VideoCreateResult + let privateVideo: VideoCreateResult // --------------------------------------------------------------- @@ -30,6 +31,7 @@ describe('Test video captions API validator', function () { await setAccessTokensToServers([ server ]) video = await server.videos.upload() + privateVideo = await server.videos.upload({ attributes: { privacy: VideoPrivacy.PRIVATE } }) { const user = { @@ -204,8 +206,32 @@ describe('Test video captions API validator', function () { }) }) + it('Should fail with a private video without token', async function () { + await makeGetRequest({ + url: server.url, + path: path + privateVideo.shortUUID + '/captions', + expectedStatus: HttpStatusCode.UNAUTHORIZED_401 + }) + }) + + it('Should fail with another user token', async function () { + await makeGetRequest({ + url: server.url, + token: userAccessToken, + path: path + privateVideo.shortUUID + '/captions', + expectedStatus: HttpStatusCode.FORBIDDEN_403 + }) + }) + it('Should success with the correct parameters', async function () { await makeGetRequest({ url: server.url, path: path + video.shortUUID + '/captions', expectedStatus: HttpStatusCode.OK_200 }) + + await makeGetRequest({ + url: server.url, + path: path + privateVideo.shortUUID + '/captions', + token: server.accessToken, + expectedStatus: HttpStatusCode.OK_200 + }) }) }) -- cgit v1.2.3