Skip to content

Commit

Permalink
Correctly check import target URL IP
Browse files Browse the repository at this point in the history
  • Loading branch information
Chocobozzz committed Feb 7, 2022
1 parent 4afec73 commit f33e515
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 13 deletions.
Expand Up @@ -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'

Expand Down
29 changes: 29 additions & 0 deletions 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<string[]>((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
}
18 changes: 7 additions & 11 deletions 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'
Expand Down Expand Up @@ -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.'
})
}
}

Expand Down
3 changes: 2 additions & 1 deletion server/tests/api/check-params/video-imports.ts
Expand Up @@ -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) {
Expand Down
17 changes: 17 additions & 0 deletions 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
})
})
1 change: 1 addition & 0 deletions server/tests/helpers/index.ts
@@ -1,5 +1,6 @@
import './image'
import './core-utils'
import './dns'
import './comment-model'
import './markdown'
import './request'

0 comments on commit f33e515

Please sign in to comment.