From 9f387884688da7db96255c8c9f0c420084ec417c Mon Sep 17 00:00:00 2001
From: Mikael Finstad <finstaden@gmail.com>
Date: Tue, 4 Jan 2022 17:02:44 +0700
Subject: [PATCH] improve private ip check (#3403)

https://huntr.dev/bounties/c1c03ef6-3f18-4976-a9ad-08c251279122/
---
 package.json                  |  2 +-
 src/server/helpers/request.js | 84 +++++------------------------------
 2 files changed, 11 insertions(+), 75 deletions(-)

diff --git a/package.json b/package.json
index de1701087b..153c72799a 100644
--- a/package.json
+++ b/package.json
@@ -45,7 +45,7 @@
     "express-session": "1.17.1",
     "grant": "4.7.0",
     "helmet": "^4.6.0",
-    "ip-address": "6.2.0",
+    "ipaddr.js": "^2.0.1",
     "isobject": "3.0.1",
     "jsonwebtoken": "8.5.1",
     "lodash.merge": "^4.6.2",
diff --git a/src/server/helpers/request.js b/src/server/helpers/request.js
index bdd5bb4a23..48125a9682 100644
--- a/src/server/helpers/request.js
+++ b/src/server/helpers/request.js
@@ -1,83 +1,19 @@
+// eslint-disable-next-line max-classes-per-file
 const http = require('http')
 const https = require('https')
 const { URL } = require('url')
 const dns = require('dns')
-const ipAddress = require('ip-address')
 const request = require('request')
+const ipaddr = require('ipaddr.js')
+
 const logger = require('../logger')
 
 const FORBIDDEN_IP_ADDRESS = 'Forbidden IP address'
 
-function isIPAddress (address) {
-  const addressAsV6 = new ipAddress.Address6(address)
-  const addressAsV4 = new ipAddress.Address4(address)
-  return addressAsV6.isValid() || addressAsV4.isValid()
-}
-
-/* eslint-disable max-len */
-/**
- * Determine if a IP address provided is a private one.
- * Return TRUE if it's the case, FALSE otherwise.
- * Excerpt from:
- * https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html#case-2---application-can-send-requests-to-any-external-ip-address-or-domain-name
- *
- * @param {string} ipAddress the ip address to validate
- * @returns {boolean}
- */
-/* eslint-enable max-len */
-function isPrivateIP (ipAddress) {
-  let isPrivate = false
-  // Build the list of IP prefix for V4 and V6 addresses
-  const ipPrefix = []
-  // Add prefix for loopback addresses
-  ipPrefix.push('127.')
-  ipPrefix.push('0.')
-  // Add IP V4 prefix for private addresses
-  // See https://en.wikipedia.org/wiki/Private_network
-  ipPrefix.push('10.')
-  ipPrefix.push('172.16.')
-  ipPrefix.push('172.17.')
-  ipPrefix.push('172.18.')
-  ipPrefix.push('172.19.')
-  ipPrefix.push('172.20.')
-  ipPrefix.push('172.21.')
-  ipPrefix.push('172.22.')
-  ipPrefix.push('172.23.')
-  ipPrefix.push('172.24.')
-  ipPrefix.push('172.25.')
-  ipPrefix.push('172.26.')
-  ipPrefix.push('172.27.')
-  ipPrefix.push('172.28.')
-  ipPrefix.push('172.29.')
-  ipPrefix.push('172.30.')
-  ipPrefix.push('172.31.')
-  ipPrefix.push('192.168.')
-  ipPrefix.push('169.254.')
-  // Add IP V6 prefix for private addresses
-  // See https://en.wikipedia.org/wiki/Unique_local_address
-  // See https://en.wikipedia.org/wiki/Private_network
-  // See https://simpledns.com/private-ipv6
-  ipPrefix.push('fc')
-  ipPrefix.push('fd')
-  ipPrefix.push('fe')
-  ipPrefix.push('ff')
-  ipPrefix.push('::1')
-  // Verify the provided IP address
-  // Remove whitespace characters from the beginning/end of the string
-  // and convert it to lower case
-  // Lower case is for preventing any IPV6 case bypass using mixed case
-  // depending on the source used to get the IP address
-  const ipToVerify = ipAddress.trim().toLowerCase()
-  // Perform the check against the list of prefix
-  for (const prefix of ipPrefix) {
-    if (ipToVerify.startsWith(prefix)) {
-      isPrivate = true
-      break
-    }
-  }
-
-  return isPrivate
-}
+// Example scary IPs that should return false (ipv6-to-ipv4 mapped):
+// ::FFFF:127.0.0.1
+// ::ffff:7f00:1
+const isDisallowedIP = (ipAddress) => ipaddr.parse(ipAddress).range() !== 'unicast'
 
 module.exports.FORBIDDEN_IP_ADDRESS = FORBIDDEN_IP_ADDRESS
 
@@ -115,7 +51,7 @@ function dnsLookup (hostname, options, callback) {
 
     const toValidate = Array.isArray(addresses) ? addresses : [{ address: addresses }]
     for (const record of toValidate) {
-      if (isPrivateIP(record.address)) {
+      if (isDisallowedIP(record.address)) {
         callback(new Error(FORBIDDEN_IP_ADDRESS), addresses, maybeFamily)
         return
       }
@@ -127,7 +63,7 @@ function dnsLookup (hostname, options, callback) {
 
 class HttpAgent extends http.Agent {
   createConnection (options, callback) {
-    if (isIPAddress(options.host) && isPrivateIP(options.host)) {
+    if (ipaddr.isValid(options.host) && isDisallowedIP(options.host)) {
       callback(new Error(FORBIDDEN_IP_ADDRESS))
       return undefined
     }
@@ -138,7 +74,7 @@ class HttpAgent extends http.Agent {
 
 class HttpsAgent extends https.Agent {
   createConnection (options, callback) {
-    if (isIPAddress(options.host) && isPrivateIP(options.host)) {
+    if (ipaddr.isValid(options.host) && isDisallowedIP(options.host)) {
       callback(new Error(FORBIDDEN_IP_ADDRESS))
       return undefined
     }