Prevent information leak

[Sampaguitas]

…URL redirects to another domain (information leak) #137

[FGRibreau]

@Sampaguitas sadly the PR was not passing the test suite. I've updated the code (see 95e7a3b#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346R34 and last commit) but I cannot publish this, because new tests are required to matching your information leak report and I was not able to reproduce it:

it('should not forward authorization headers when the request has a redirect', function (done) {
    var r = request.defaults({
      json: true
    });
    r('https://httpbingo.org/redirect-to?status_code=301&url=https://httpbingo.org/get', function (err, response, body) {
      t.strictEqual(body.headers.authorization, ''); // even without your PR merge, authorization header is still empty
      done();
    }).auth('user', 'passwd', false);
  });

/cc @JamieSlome

[Sampaguitas]

@FGRibreau

Your aws4 dependency is removing the authorization from the header but not the cookie (as mentioned in the report).

Both your "redirect url" and "original url" are the same (protocol://host:port), my function only trims the cookie from the header if at least one of the 3 parameters (protocol, host or port) is different...

Try this test, it worked like a charm:

//leak.test.js
'use strict';

var request = require('../').defaults({ json: true });;
var t = require('chai').assert;

describe('Information Leak', function () {

    it('should not forward cookie headers when the request has a redirect', function (done) {

        request({
            url: 'https://httpbingo.org/cookies?url=https://google.com/',
            headers: {
                'Content-Type': 'application/json',
                'cookie': 'ajs_anonymous_id=1234567890',
                'authorization': 'Bearer eyJhb12345abcdef'
            }
        }, function (err, response, body) {
            t.strictEqual(Object.keys(body).length, 0);
            done();
        });
    });

    it('should not forward authorization headers when the request has a redirect', function (done) {

        request({
            url: 'https://httpbingo.org/bearer?url=https://google.com/',
            headers: {
                'Content-Type': 'application/json',
                'cookie': 'ajs_anonymous_id=1234567890',
                'authorization': 'Bearer eyJhb12345abcdef'
            }
        }, function (err, response, body) {
            t.strictEqual(body, undefined);
            done();
        });
    });
});

I have submitted a new PR with the test file: #139

Thanks for sharing this awesome testing API.

/cc @JamieSlome

[FGRibreau]

weird, both https://httpbingo.org/bearer?url= & https://httpbingo.org/cookies?url= do not do a redirect so that test is not valid, am I right?

In order to test the information leak we would have to send a request with authorization headers and that request would tell node-request-reply through a location header to redirect to another domain and then node-request-reply would send these authorization header to the new domain.

If none of these happen then there is no information leak right? So the test has to check that

[Sampaguitas]

Have you tested the solution with redirect?

https://httpbingo.org/redirect-to?url=http://httpbingo.org/cookies
https://httpbingo.org/redirect-to?url=http://httpbingo.org/bearer

See message here.