CVE-2024-29041 (JavaScript, published 2024-03-25T21:15Z)
Description: value: Express.js minimalist web framework for node. Versions of Express.js prior to 4.19.0 and all pre-release alpha and beta versions of 5.0 are affected by an open redirect vulnerability using malformed URLs. When a user of Express performs a redirect using a user-provided URL Express performs an encode [using `encodeurl`](https://github.com/pillarjs/encodeurl) on the contents before passing it to the `location` header. This can cause malformed URLs to be evaluated in unexpected ways by common redirect allow list implementations in Express applications, leading to an Open Redirect via bypass of a properly implemented allow list. The main method impacted is `res.location()` but this is also called from within `res.redirect()`. The vulnerability is fixed in 4.19.2 and 5.0.0-beta.3.
Severity: nan (CVSS v3 base score: )
Weakness: NVD-CWE-noinfo \u2014 Insufficient Information
There is insufficient information about the issue to classify it; details are unkown or unspecified.
Repository: https://github.com/expressjs/express
Files changed:
History.md
lib/response.js
test/res.location.js
Commit message: Improved fix for open redirect allow list bypass

Co-authored-by: Jon Church <me@jonchurch.com>
Co-authored-by: Blake Embrey <hello@blakeembrey.com>


Vulnerable code:
var urlParse = require('url').parse;
  var loc = String(url);
  var lowerLoc = loc.toLowerCase();
  var encodedUrl = encodeUrl(loc);
  if (lowerLoc.indexOf('https://') === 0 || lowerLoc.indexOf('http://') === 0) {
    try {
      var parsedUrl = urlParse(loc);
      var parsedEncodedUrl = urlParse(encodedUrl);
      // Because this can encode the host, check that we did not change the host
      if (parsedUrl.host !== parsedEncodedUrl.host) {
        // If the host changes after encodeUrl, return the original url
        return this.set('Location', loc);
      }
    } catch (e) {
      // If parse fails, return the original url
      return this.set('Location', loc);
    }
  }
  // set location
  return this.set('Location', encodedUrl);
    it('should not encode bad "url"', function (done) {
      var app = express()

      app.use(function (req, res) {
        // This is here to show a basic check one might do which
        // would pass but then the location header would still be bad
        if (url.parse(req.query.q).host !== 'google.com') {
          res.status(400).end('Bad url');
        }
        res.location(req.query.q).end();
      });

      request(app)
        .get('/?q=http://google.com' + encodeURIComponent('\\@apple.com'))
        .expect(200)
        .expect('Location', 'http://google.com\\@apple.com')
        .end(function (err) {
          if (err) {
            throw err;
          }

          // This ensures that our protocol check is case insensitive
          request(app)
            .get('/?q=HTTP://google.com' + encodeURIComponent('\\@apple.com'))
            .expect(200)
            .expect('Location', 'HTTP://google.com\\@apple.com')
            .end(done)
        });
    });

    it('should not touch already-encoded sequences in "url"', function (done) {
      var app = express()

      app.use(function (req, res) {
        res.location('https://google.com?q=%A710').end()
      })

      request(app)
      .get('/')
      .expect('Location', 'https://google.com?q=%A710')
      .expect(200, done)
    })


Fixed code:
unreleased
==========

  * Improved fix for open redirect allow list bypass

var schemaAndHostRegExp = /^(?:[a-zA-Z][a-zA-Z0-9+.-]*:)?\/\/[^\\\/\?]+/;
  var loc;
  } else {
    loc = String(url);
  var m = schemaAndHostRegExp.exec(loc);
  var pos = m ? m[0].length + 1 : 0;

  // Only encode after host to avoid invalid encoding which can introduce
  // vulnerabilities (e.g. `\\` to `%5C`).
  loc = loc.slice(0, pos) + encodeUrl(loc.slice(pos));
  return this.set('Location', loc);
  , assert = require('assert')
    it('should encode data uri', function (done) {
      var app = express()
      app.use(function (req, res) {
        res.location('data:text/javascript,export default () => { }').end();
      });

      request(app)
        .get('/')
        .expect('Location', 'data:text/javascript,export%20default%20()%20=%3E%20%7B%20%7D')
        .expect(200, done)
    })

    it('should encode data uri', function (done) {
      var app = express()
      app.use(function (req, res) {
        res.location('data:text/javascript,export default () => { }').end();
      });

      request(app)
        .get('/')
        .expect('Location', 'data:text/javascript,export%20default%20()%20=%3E%20%7B%20%7D')
        .expect(200, done)
    })

    it('should consistently handle non-string input: boolean', function (done) {
      var app = express()
      app.use(function (req, res) {
        res.location(true).end();
      });

      request(app)
        .get('/')
        .expect('Location', 'true')
        .expect(200, done)
    });

    it('should consistently handle non-string inputs: object', function (done) {
      var app = express()
      app.use(function (req, res) {
        res.location({}).end();
      });

      request(app)
        .get('/')
        .expect('Location', '[object%20Object]')
        .expect(200, done)
    });

    it('should consistently handle non-string inputs: array', function (done) {
      var app = express()
      app.use(function (req, res) {
        res.location([]).end();
      });

      request(app)
        .get('/')
        .expect('Location', '')
        .expect(200, done)
    });

    it('should consistently handle empty string input', function (done) {
      var app = express()
      app.use(function (req, res) {
        res.location('').end();
      });

      request(app)
        .get('/')
        .expect('Location', '')
        .expect(200, done)
    });



  describe('location header encoding', function() {
    function createRedirectServerForDomain (domain) {
      var app = express();
      app.use(function (req, res) {
        var host = url.parse(req.query.q, false, true).host;
        // This is here to show a basic check one might do which
        // would pass but then the location header would still be bad
        if (host !== domain) {
          res.status(400).end('Bad host: ' + host + ' !== ' + domain);
        }
        res.location(req.query.q).end();
      });
      return app;
    }

    function testRequestedRedirect (app, inputUrl, expected, expectedHost, done) {
      return request(app)
        // Encode uri because old supertest does not and is required
        // to test older node versions. New supertest doesn't re-encode
        // so this works in both.
        .get('/?q=' + encodeURIComponent(inputUrl))
        .expect('') // No body.
        .expect(200)
        .expect('Location', expected)
        .end(function (err, res) {
          if (err) {
            console.log('headers:', res.headers)
            console.error('error', res.error, err);
            return done(err, res);
          }

          // Parse the hosts from the input URL and the Location header
          var inputHost = url.parse(inputUrl, false, true).host;
          var locationHost = url.parse(res.headers['location'], false, true).host;

          assert.strictEqual(locationHost, expectedHost);

          // Assert that the hosts are the same
          if (inputHost !== locationHost) {
            return done(new Error('Hosts do not match: ' + inputHost + " !== " +  locationHost));
          }

          return done(null, res);
        });
    }

    it('should not touch already-encoded sequences in "url"', function (done) {
      var app = createRedirectServerForDomain('google.com');
      testRequestedRedirect(
        app,
        'https://google.com?q=%A710',
        'https://google.com?q=%A710',
        'google.com',
        done
      );
    });

    it('should consistently handle relative urls', function (done) {
      var app = createRedirectServerForDomain(null);
      testRequestedRedirect(
        app,
        '/foo/bar',
        '/foo/bar',
        null,
        done
      );
    });

    it('should not encode urls in such a way that they can bypass redirect allow lists', function (done) {
      var app = createRedirectServerForDomain('google.com');
      testRequestedRedirect(
        app,
        'http://google.com\\@apple.com',
        'http://google.com\\@apple.com',
        'google.com',
        done
      );
    });

    it('should not be case sensitive', function (done) {
      var app = createRedirectServerForDomain('google.com');
      testRequestedRedirect(
        app,
        'HTTP://google.com\\@apple.com',
        'HTTP://google.com\\@apple.com',
        'google.com',
        done
      );
    });

    it('should work with https', function (done) {
      var app = createRedirectServerForDomain('google.com');
      testRequestedRedirect(
        app,
        'https://google.com\\@apple.com',
        'https://google.com\\@apple.com',
        'google.com',
        done
      );
    });

    it('should correctly encode schemaless paths', function (done) {
      var app = createRedirectServerForDomain('google.com');
      testRequestedRedirect(
        app,
        '//google.com\\@apple.com/',
        '//google.com\\@apple.com/',
        'google.com',
        done
      );
    });

    it('should percent encode backslashes in the path', function (done) {
      var app = createRedirectServerForDomain('google.com');
      testRequestedRedirect(
        app,
        'https://google.com/foo\\bar\\baz',
        'https://google.com/foo%5Cbar%5Cbaz',
        'google.com',
        done
      );
    });

    it('should encode backslashes in the path after the first backslash that triggered path parsing', function (done) {
      var app = createRedirectServerForDomain('google.com');
      testRequestedRedirect(
        app,
        'https://google.com\\@app\\l\\e.com',
        'https://google.com\\@app%5Cl%5Ce.com',
        'google.com',
        done
      );
    });

    it('should escape header splitting for old node versions', function (done) {
      var app = createRedirectServerForDomain('google.com');
      testRequestedRedirect(
        app,
        'http://google.com\\@apple.com/%0d%0afoo:%20bar',
        'http://google.com\\@apple.com/%0d%0afoo:%20bar',
        'google.com',
        done
      );
    });

    it('should encode unicode correctly', function (done) {
      var app = createRedirectServerForDomain(null);
      testRequestedRedirect(
        app,
        '/%e2%98%83',
        '/%e2%98%83',
        null,
        done
      );
    });

    it('should encode unicode correctly even with a bad host', function (done) {
      var app = createRedirectServerForDomain('google.com');
      testRequestedRedirect(
        app,
        'http://google.com\\@apple.com/%e2%98%83',
        'http://google.com\\@apple.com/%e2%98%83',
        'google.com',
        done
      );
    });

    it('should work correctly despite using deprecated url.parse', function (done) {
      var app = createRedirectServerForDomain('google.com');
      testRequestedRedirect(
        app,
        'https://google.com\'.bb.com/1.html',
        'https://google.com\'.bb.com/1.html',
        'google.com',
        done
      );
    });

    it('should encode file uri path', function (done) {
      var app = createRedirectServerForDomain('');
      testRequestedRedirect(
        app,
        'file:///etc\\passwd',
        'file:///etc%5Cpasswd',
        '',
        done
      );
    });
  });