fix: prevent host header injection by sanitizing userinfo in Host header

Author: bjohansebas

lib/request.js

@@ -22,6 +22,23 @@ var parseRange = require('range-parser');
 var parse = require('parseurl');
 var proxyaddr = require('proxy-addr');
 
+/**
+ * Determine whether a string is a valid port, i.e. only ASCII digits,
+ * per RFC 3986 section 3.2.3 ("port = *DIGIT").
+ *
+ * @param {string} str
+ * @return {boolean}
+ * @private
+ */
+
+function isValidPort (str) {
+  for (var i = 0; i < str.length; i++) {
+    var code = str.charCodeAt(i);
+    if (code < 0x30 || code > 0x39) return false;
+  }
+  return true;
+}
+
 /**
  * Request prototype.
  * @public
@@ -427,7 +444,25 @@ defineGetter(req, 'host', function host(){
     val = val.substring(0, val.indexOf(',')).trimEnd()
   }
 
-  return val || undefined;
+  if (!val) return;
+
+  // A Host header must not contain userinfo (RFC 9110 section 7.2). Drop any
+  // "user@" / "user:pass@" prefix so a crafted value like
+  // "evil.com:x@good.com" cannot inject an attacker-controlled host via
+  // req.hostname; the real host is the part after the last "@".
+  var atIndex = val.lastIndexOf('@');
+  if (atIndex !== -1) val = val.slice(atIndex + 1);
+
+  if (!val) return;
+
+  // The optional port must be numeric (RFC 3986 section 3.2.3). Reject
+  // malformed authorities, e.g. an encoded "@" smuggled in as the port
+  // ("evil.com:x%40good.com"), which would otherwise leak a fake hostname.
+  var portOffset = val[0] === '[' ? val.indexOf(']') + 1 : 0;
+  var portIndex = val.indexOf(':', portOffset);
+  if (portIndex !== -1 && !isValidPort(val.slice(portIndex + 1))) return;
+
+  return val;
 });
 
 /**

test/req.host.js

@@ -70,6 +70,34 @@ describe('req', function(){
       .expect(200, '[::1]:3000', done);
     })
 
+    describe('with a Host header containing userinfo', function(){
+      it('should return the real host, dropping the userinfo', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(req.host);
+        });
+
+        request(app)
+        .post('/')
+        .set('Host', 'evil.com:fake@legitimate.com:3000')
+        .expect(200, 'legitimate.com:3000', done);
+      })
+
+      it('should return undefined for an encoded userinfo separator', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(String(req.host));
+        });
+
+        request(app)
+        .post('/')
+        .set('Host', 'evil.com:x%40legitimate.com')
+        .expect(200, 'undefined', done);
+      })
+    })
+
     describe('when "trust proxy" is enabled', function(){
       it('should respect X-Forwarded-Host', function(done){
         var app = express();

test/req.hostname.js

@@ -70,6 +70,125 @@ describe('req', function(){
       .expect('[::1]', done);
     })
 
+    it('should return undefined for a non-numeric port', function(done){
+      var app = express();
+
+      app.use(function(req, res){
+        res.end(String(req.hostname));
+      });
+
+      request(app)
+      .post('/')
+      .set('Host', 'example.com:notaport')
+      .expect('undefined', done);
+    })
+
+    it('should return undefined for an IPv6 Host with a non-numeric port', function(done){
+      var app = express();
+
+      app.use(function(req, res){
+        res.end(String(req.hostname));
+      });
+
+      request(app)
+      .post('/')
+      .set('Host', '[::1]:notaport')
+      .expect('undefined', done);
+    })
+
+    describe('with a Host header containing userinfo', function(){
+      it('should return the real host, not the userinfo prefix', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(req.hostname);
+        });
+
+        request(app)
+        .post('/')
+        .set('Host', 'evil.com:fake@legitimate.com:3000')
+        .expect('legitimate.com', done);
+      })
+
+      it('should ignore a userinfo section before the host', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(req.hostname);
+        });
+
+        request(app)
+        .post('/')
+        .set('Host', 'user@example.com')
+        .expect('example.com', done);
+      })
+
+      it('should strip the port after removing userinfo', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(req.hostname);
+        });
+
+        request(app)
+        .post('/')
+        .set('Host', 'user:pass@example.com:8080')
+        .expect('example.com', done);
+      })
+
+      it('should remove userinfo from an IPv6 Host', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(req.hostname);
+        });
+
+        request(app)
+        .post('/')
+        .set('Host', 'user@[::1]:8080')
+        .expect('[::1]', done);
+      })
+
+      it('should use the host after the last "@" when several are present', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(req.hostname);
+        });
+
+        request(app)
+        .post('/')
+        .set('Host', 'a@b@example.com')
+        .expect('example.com', done);
+      })
+
+      it('should return undefined for an encoded userinfo separator', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(String(req.hostname));
+        });
+
+        request(app)
+        .post('/')
+        .set('Host', 'evil.com:x%40legitimate.com')
+        .expect('undefined', done);
+      })
+
+      it('should return undefined when there is no host after the userinfo', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(String(req.hostname));
+        });
+
+        request(app)
+        .post('/')
+        .set('Host', 'user@')
+        .expect('undefined', done);
+      })
+    })
+
     describe('when "trust proxy" is enabled', function(){
       it('should respect X-Forwarded-Host', function(done){
         var app = express();
@@ -87,6 +206,22 @@ describe('req', function(){
         .expect('example.com', done);
       })
 
+      it('should drop userinfo from a trusted X-Forwarded-Host', function(done){
+        var app = express();
+
+        app.enable('trust proxy');
+
+        app.use(function(req, res){
+          res.end(req.hostname);
+        });
+
+        request(app)
+        .get('/')
+        .set('Host', 'localhost')
+        .set('X-Forwarded-Host', 'evil.com:fake@legitimate.com')
+        .expect('legitimate.com', done);
+      })
+
       it('should ignore X-Forwarded-Host if socket addr not trusted', function(done){
         var app = express();