CVE-2022-0508: Server-Side Request Forgery (SSRF) (CWE-918)
value: Server-Side Request Forgery (SSRF) in GitHub repository chocobozzz/peertube prior to f33e515991a32885622b217bf2ed1d1b0d9d6832
CWE description: The web server receives a URL or similar request from an upstream component and retrieves the contents of this URL, but it does not sufficiently ensure that the request is being sent to the expected destination.
CVSS v3 base score: 5.3
Published: 2022-02-08T11:15Z
Repository: https://github.com/chocobozzz/peertube
Language: TypeScript
Files changed: client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts, server/helpers/dns.ts, server/middlewares/validators/videos/video-imports.ts, server/tests/api/check-params/video-imports.ts, server/tests/helpers/dns.ts, server/tests/helpers/index.ts
Commit date: 2022-02-07 11:21:25 +0100
Commit message: Correctly check import target URL IP

Diff: 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 4e77cac475..fb6f2601b3 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
@@ -7,11 +7,11 @@ import { Router } from '@angular/router'
 import { AuthService, CanComponentDeactivate, HooksService, Notifier, ServerService, UserService } from '@app/core'
 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'
 
 @Component({
   selector: 'my-video-upload',
diff --git a/server/helpers/dns.ts b/server/helpers/dns.ts
new file mode 100644
index 0000000000..da8b666c2d
--- /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<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
+}
diff --git a/server/middlewares/validators/videos/video-imports.ts b/server/middlewares/validators/videos/video-imports.ts
index a3a5cc5315..9c6d213c42 100644
--- a/server/middlewares/validators/videos/video-imports.ts
+++ b/server/middlewares/validators/videos/video-imports.ts
@@ -1,8 +1,8 @@
 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'
 import { HttpStatusCode, UserRight, VideoImportState } from '@shared/models'
 import { VideoImportCreate } from '@shared/models/videos/import/video-import-create.model'
@@ -74,21 +74,17 @@ 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.'
+        })
       }
     }
 
     if (!await isImportAccepted(req, res)) return cleanUpReqFiles(req)
 
diff --git a/server/tests/api/check-params/video-imports.ts b/server/tests/api/check-params/video-imports.ts
index 156a612ee7..7893f5cc58 100644
--- a/server/tests/api/check-params/video-imports.ts
+++ b/server/tests/api/check-params/video-imports.ts
@@ -118,11 +118,12 @@ describe('Test video imports API validator', 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'
+        'http://192.168.1.42',
+        'http://127.0.0.1.cpy.re'
       ]
 
       for (const targetUrl of targetUrls) {
         const fields = { ...baseCorrectParams, targetUrl }
 
diff --git a/server/tests/helpers/dns.ts b/server/tests/helpers/dns.ts
new file mode 100644
index 0000000000..309de5426f
--- /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 91d11e25db..951208842c 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'

Vulnerable code: import { HttpStatusCode, VideoCreateResult, VideoPrivacy } from '@shared/models'
import { isValid as isIPValid, parse as parseIP } from 'ipaddr.js'
      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.'
          })
        }
        'http://192.168.1.42'
Fixed code: import { HttpStatusCode, VideoCreateResult } from '@shared/models'
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
}
import { isResolvingToUnicastOnly } from '@server/helpers/dns'
      if (await isResolvingToUnicastOnly(hostname) !== true) {
        cleanUpReqFiles(req)
        return res.fail({
          status: HttpStatusCode.FORBIDDEN_403,
          message: 'Cannot use non unicast IP as targetUrl.'
        })
        'http://192.168.1.42',
        'http://127.0.0.1.cpy.re'
/* 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
  })
})
import './dns'