From f33e515991a32885622b217bf2ed1d1b0d9d6832 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Mon, 7 Feb 2022 11:21:25 +0100 Subject: [PATCH] Correctly check import target URL IP --- .../video-upload.component.ts | 2 +- server/helpers/dns.ts | 29 +++++++++++++++++++ .../validators/videos/video-imports.ts | 18 +++++------- .../tests/api/check-params/video-imports.ts | 3 +- server/tests/helpers/dns.ts | 17 +++++++++++ server/tests/helpers/index.ts | 1 + 6 files changed, 57 insertions(+), 13 deletions(-) create mode 100644 server/helpers/dns.ts create mode 100644 server/tests/helpers/dns.ts diff --git a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts index 4e77cac47..fb6f2601b 100644 --- a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts +++ b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts @@ -9,7 +9,7 @@ import { genericUploadErrorHandler, scrollToTop } from '@app/helpers' import { FormValidatorService } from '@app/shared/shared-forms' import { BytesPipe, Video, VideoCaptionService, VideoEdit, VideoService } from '@app/shared/shared-main' import { LoadingBarService } from '@ngx-loading-bar/core' -import { HttpStatusCode, VideoCreateResult, VideoPrivacy } from '@shared/models' +import { HttpStatusCode, VideoCreateResult } from '@shared/models' import { UploaderXFormData } from './uploaderx-form-data' import { VideoSend } from './video-send' diff --git a/server/helpers/dns.ts b/server/helpers/dns.ts new file mode 100644 index 000000000..da8b666c2 --- /dev/null +++ b/server/helpers/dns.ts @@ -0,0 +1,29 @@ +import { lookup } from 'dns' +import { parse as parseIP } from 'ipaddr.js' + +function dnsLookupAll (hostname: string) { + return new Promise((res, rej) => { + lookup(hostname, { family: 0, all: true }, (err, adresses) => { + if (err) return rej(err) + + return res(adresses.map(a => a.address)) + }) + }) +} + +async function isResolvingToUnicastOnly (hostname: string) { + const addresses = await dnsLookupAll(hostname) + + for (const address of addresses) { + const parsed = parseIP(address) + + if (parsed.range() !== 'unicast') return false + } + + return true +} + +export { + dnsLookupAll, + isResolvingToUnicastOnly +} diff --git a/server/middlewares/validators/videos/video-imports.ts b/server/middlewares/validators/videos/video-imports.ts index a3a5cc531..9c6d213c4 100644 --- a/server/middlewares/validators/videos/video-imports.ts +++ b/server/middlewares/validators/videos/video-imports.ts @@ -1,6 +1,6 @@ import express from 'express' import { body, param } from 'express-validator' -import { isValid as isIPValid, parse as parseIP } from 'ipaddr.js' +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' @@ -76,17 +76,13 @@ const videoImportAddValidator = getCommonVideoEditAttributes().concat([ if (req.body.targetUrl) { const hostname = new URL(req.body.targetUrl).hostname - if (isIPValid(hostname)) { - const parsed = parseIP(hostname) + if (await isResolvingToUnicastOnly(hostname) !== true) { + cleanUpReqFiles(req) - if (parsed.range() !== 'unicast') { - cleanUpReqFiles(req) - - return res.fail({ - status: HttpStatusCode.FORBIDDEN_403, - message: 'Cannot use non unicast IP as targetUrl.' - }) - } + return res.fail({ + status: HttpStatusCode.FORBIDDEN_403, + message: 'Cannot use non unicast IP as targetUrl.' + }) } } diff --git a/server/tests/api/check-params/video-imports.ts b/server/tests/api/check-params/video-imports.ts index 156a612ee..7893f5cc5 100644 --- a/server/tests/api/check-params/video-imports.ts +++ b/server/tests/api/check-params/video-imports.ts @@ -120,7 +120,8 @@ describe('Test video imports API validator', function () { 'http://127.0.0.1', 'http://127.0.0.1/hello', 'https://192.168.1.42', - 'http://192.168.1.42' + 'http://192.168.1.42', + 'http://127.0.0.1.cpy.re' ] for (const targetUrl of targetUrls) { diff --git a/server/tests/helpers/dns.ts b/server/tests/helpers/dns.ts new file mode 100644 index 000000000..309de5426 --- /dev/null +++ b/server/tests/helpers/dns.ts @@ -0,0 +1,17 @@ +/* eslint-disable @typescript-eslint/no-unused-expressions,@typescript-eslint/require-await */ + +import 'mocha' +import { expect } from 'chai' +import { isResolvingToUnicastOnly } from '@server/helpers/dns' + +describe('DNS helpers', function () { + + it('Should correctly check unicast IPs', async function () { + expect(await isResolvingToUnicastOnly('cpy.re')).to.be.true + expect(await isResolvingToUnicastOnly('framasoft.org')).to.be.true + expect(await isResolvingToUnicastOnly('8.8.8.8')).to.be.true + + expect(await isResolvingToUnicastOnly('127.0.0.1')).to.be.false + expect(await isResolvingToUnicastOnly('127.0.0.1.cpy.re')).to.be.false + }) +}) diff --git a/server/tests/helpers/index.ts b/server/tests/helpers/index.ts index 91d11e25d..951208842 100644 --- a/server/tests/helpers/index.ts +++ b/server/tests/helpers/index.ts @@ -1,5 +1,6 @@ import './image' import './core-utils' +import './dns' import './comment-model' import './markdown' import './request'