From 6f4247ae7ae3b7042ae874524c8f6ccc4ee433fb Mon Sep 17 00:00:00 2001
From: wica-sufatmawati <116701588+wica-sufatmawati@users.noreply.github.com>
Date: Tue, 16 Jan 2024 06:20:11 +0700
Subject: [PATCH] Update and Fixed Improper Input Validation Follow Redirects
 improperly handles URLs in the url.parse() function

## Summary
Affected of this project `concretecms/concretecms` are vulnerable to Improper Input Validation due to the improper handling of URLs by the url.parse() function. When new URL() throws an error, it can be manipulated to misinterpret the hostname. An attacker could exploit this weakness to redirect traffic to a malicious site, potentially leading to information disclosure, phishing attacks, or other security breaches.

**PoC**
```js
# Case 1 : Bypassing localhost restriction
let url = 'http://[localhost]/admin';
try{
    new URL(url); // ERROR : Invalid URL
}catch{
    url.parse(url); // -> http://localhost/admin
}

# Case 2 : Bypassing domain restriction
let url = 'http://attacker.domain*.allowed.domain:a';
try{
    new URL(url); // ERROR : Invalid URL
}catch{
    url.parse(url); // -> http://attacker.domain/*.allowed.domain:a
}
```
Below is part of follow-redirects's index.js code.
```
function urlToOptions(urlObject) {
  var options = {
    protocol: urlObject.protocol,
    hostname: urlObject.hostname.startsWith("[") ?
      /* istanbul ignore next */
      urlObject.hostname.slice(1, -1) :
      urlObject.hostname,
    hash: urlObject.hash,
    search: urlObject.search,
    pathname: urlObject.pathname,
    path: urlObject.pathname + urlObject.search,
    href: urlObject.href,
  };
  if (urlObject.port !== "") {
    options.port = Number(urlObject.port);
  }
  return options;
}
```
It checks URL hostname which is startswith [ character. Which means if the urlObject is `http://[localhost]/`, then it converts to `http://localhost/`.

But actually above code is not vulnerable code.
(Just the idea comes from above code)

The problem comes from below code.
```
    function request(input, options, callback) {
      // Parse parameters
      if (isString(input)) {
        var parsed;
        try {
          parsed = urlToOptions(new URL(input));
        }
        catch (err) {
          /* istanbul ignore next */
          parsed = url.parse(input);
        }
    /* below code skipped */
```
**Proof of Concept**
```js
// PoC.js
const express = require("express");
const http = require('follow-redirects').http;
const app  = express();
const port = 80;

const isLocalhost = function (url) {
    try{
        u = new URL(url);
        if(u.hostname.includes('localhost')
        || u.hostname.includes('127.0.0.1')
        ){
            return true;
        }
    }catch{}
    return false;
}
app.use(express.json())

app.get("/", (req, res) => {
    res.send("Hello World");
})

app.post("/request", (req, res) => {
    let url = req.body.url;
    let options = {
        'followRedirects':false
    }
    if(req.body?.url){
        if(isLocalhost(req.body.url)){
            res.send('Localhost is restricted.');
            return;
        };
        http.get(url, options, (response) => {
            let data = '';
            response.on('data', chunk => {
                data += chunk.toString();
            });
            response.on('end', () => {
                res.send(data);
            })
        }).on('error', (e) => {
            console.log(e);
            res.status(500).send('Server Error');
        })
    }else{
        res.send('URL is required.');
    }
})

app.get("/admin", (req, res) => {
    if(req.socket.remoteAddress.includes('127.0.0.1')){
        res.send('Admin Page');
    }else{
        res.status(404).send('Not Found');
    }
})

app.listen(port, () => {
    console.log(`App listening on port ${port}`)
})
```
The main point is that inside of `folow-redirects `module, it uses vulnerable function `url.parse()` when `new URL()` throws error.

## Impact
CWE-20
CWE-601
`CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:C/C:L/I:L/A:N`
---
 build/package-lock.json | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/build/package-lock.json b/build/package-lock.json
index d393738eb5..f98e2f5652 100644
--- a/build/package-lock.json
+++ b/build/package-lock.json
@@ -6156,9 +6156,9 @@
       }
     },
     "node_modules/follow-redirects": {
-      "version": "1.15.2",
-      "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.2.tgz",
-      "integrity": "sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA==",
+      "version": "1.15.5",
+      "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.5.tgz",
+      "integrity": "sha512-vSFWUON1B+yAw1VN4xMfxgn5fTUiaOzAJCKBwIIgT/+7CuGy9+r+5gITvP62j3RmaD5Ph65UaERdOSRGUzZtgw==",
       "dev": true,
       "funding": [
         {