CVE-2020-11020: Improper Authentication (CWE-287)
value: Faye (NPM, RubyGem) versions greater than 0.5.0 and before 1.0.4, 1.1.3 and 1.2.5, has the potential for authentication bypass in the extension system. The vulnerability allows any client to bypass checks put in place by server-side extensions, by appending extra segments to the message channel. It is patched in versions 1.0.4, 1.1.3 and 1.2.5.
CWE description: When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
CVSS v3 base score: 9.8
Published: 2020-04-29T18:15Z
Repository: https://github.com/faye/faye
Language: JavaScript
Files changed: lib/faye/protocol/server.rb, spec/javascript/server/extensions_spec.js, spec/ruby/server/extensions_spec.rb, src/protocol/server.js
Commit date: 2020-04-27 21:54:29 +0100
Commit message: Strict meta channel recognition in server

This addresses a security vulnerability affecting user-added extensions
that implement access control for channels. These extensions typically
work by checking incoming messages whose channel is `/meta/subscribe`
and then performing some authentication routine before allowing the
message through.

However, the Server parses channels in a way that means any channel
namespaced under `/meta/subscribe` will also work as a subscription
request. For example if the client sends a message to the channel
`/meta/subscribe/x`, that will bypass most authentication extensions but
will still be interpreted by the server as a subscription request, and
the client will be subscribed to the requested channel. The client has
thus bypassed the user's access control policy.

Here we prevent this by using a strict equality check; only messages
whose channel is exactly `/meta/subscribe` will be interpreted as
subscription requests. The same pattern is applied to all other meta
channels.

Diff: diff --git a/lib/faye/protocol/server.rb b/lib/faye/protocol/server.rb
index 6e32a0b..96455f6 100644
--- a/lib/faye/protocol/server.rb
+++ b/lib/faye/protocol/server.rb
@@ -4,12 +4,10 @@ module Faye
     autoload :Socket, File.join(ROOT, 'faye', 'protocol', 'socket')
 
     include Logging
     include Extensible
 
-    META_METHODS = %w[handshake connect disconnect subscribe unsubscribe]
-
     attr_reader :engine
 
     def initialize(options = {})
       @options    = options || {}
       engine_opts = @options[:engine] || {}
@@ -105,13 +103,13 @@ module Faye
       response['successful'] = !response['error']
       callback.call([response])
     end
 
     def handle_meta(message, local, &callback)
-      method = Channel.parse(message['channel'])[1]
+      method = method_for(message)
 
-      unless META_METHODS.include?(method)
+      unless method
         response = make_response(message)
         response['error'] = Faye::Error.channel_forbidden(message['channel'])
         response['successful'] = false
         return callback.call([response])
       end
@@ -121,10 +119,20 @@ module Faye
         responses.each { |r| advize(r, message['connectionType']) }
         callback.call(responses)
       end
     end
 
+    def method_for(message)
+      case message['channel']
+      when Channel::HANDSHAKE   then :handshake
+      when Channel::CONNECT     then :connect
+      when Channel::SUBSCRIBE   then :subscribe
+      when Channel::UNSUBSCRIBE then :unsubscribe
+      when Channel::DISCONNECT  then :disconnect
+      end
+    end
+
     def advize(response, connection_type)
       return unless [Channel::HANDSHAKE, Channel::CONNECT].include?(response['channel'])
 
       if connection_type == 'eventsource'
         interval = (@engine.timeout * 1000).floor
diff --git a/spec/javascript/server/extensions_spec.js b/spec/javascript/server/extensions_spec.js
index 921e430..bcbcaad 100644
--- a/spec/javascript/server/extensions_spec.js
+++ b/spec/javascript/server/extensions_spec.js
@@ -34,10 +34,46 @@ jstest.describe("Server extensions", function() { with(this) {
       server.process({channel: "/meta/handshake"}, false, function(r) { response = r })
       assertEqual( [{channel: "/foo", data: "hello"}], response )
     }})
   }})
 
+  describe("with subscription auth installed", function() { with(this) {
+    before(function() { with(this) {
+      var extension = {
+        incoming: function(message, callback) {
+          if (message.channel === "/meta/subscribe" && !message.auth) {
+            message.error = "Invalid auth"
+          }
+          callback(message)
+        }
+      }
+      server.addExtension(extension)
+    }})
+
+    it("does not subscribe using the intended channel", function() { with(this) {
+      var message = {
+        channel: "/meta/subscribe",
+        clientId: "fakeclientid",
+        subscription: "/foo"
+      }
+      stub(engine, "clientExists").yields([true])
+      expect(engine, "subscribe").exactly(0)
+      server.process(message, false, function() {})
+    }})
+
+    it("does not subscribe using an extended channel", function() { with(this) {
+      var message = {
+        channel: "/meta/subscribe/x",
+        clientId: "fakeclientid",
+        subscription: "/foo"
+      }
+      stub(engine, "clientExists").yields([true])
+      expect(engine, "subscribe").exactly(0)
+      server.process(message, false, function() {})
+    }})
+  }})
+
   describe("with an outgoing extension installed", function() { with(this) {
     before(function() { with(this) {
       var extension = {
         outgoing: function(message, callback) {
           message.ext = {auth: "password"}
diff --git a/spec/ruby/server/extensions_spec.rb b/spec/ruby/server/extensions_spec.rb
index cd12fee..cfc0a7c 100644
--- a/spec/ruby/server/extensions_spec.rb
+++ b/spec/ruby/server/extensions_spec.rb
@@ -38,10 +38,46 @@ describe "server extensions" do
       server.process({"channel" => "/meta/handshake"}, false) { |r| response = r }
       response.should == [{"channel" => "/foo", "data" => "hello"}]
     end
   end
 
+  describe "with subscription auth installed" do
+    before do
+      extension = Class.new do
+        def incoming(message, callback)
+          if message["channel"] == "/meta/subscribe" and !message["auth"]
+            message["error"] = "Invalid auth"
+          end
+          callback.call(message)
+        end
+      end
+      server.add_extension(extension.new)
+    end
+
+    it "does not subscribe using the intended channel" do
+      message = {
+        "channel" => "/meta/subscribe",
+        "clientId" => "fakeclientid",
+        "subscription" => "/foo"
+      }
+      engine.stub(:client_exists).and_yield(true)
+      engine.should_not_receive(:subscribe)
+      server.process(message, false) {}
+    end
+
+    it "does not subscribe using an extended channel" do
+      message = {
+        "channel" => "/meta/subscribe/x",
+        "clientId" => "fakeclientid",
+        "subscription" => "/foo"
+      }
+      engine.stub(:client_exists).and_yield(true)
+      engine.should_not_receive(:subscribe)
+      server.process(message, false) {}
+    end
+  end
+
   describe "with an outgoing extension installed" do
     before do
       extension = Class.new do
         def outgoing(message, callback)
           message["ext"] = {"auth" => "password"}
diff --git a/src/protocol/server.js b/src/protocol/server.js
index f7a9a10..96aa6cb 100644
--- a/src/protocol/server.js
+++ b/src/protocol/server.js
@@ -11,12 +11,10 @@ var Class      = require('../util/class'),
     Extensible = require('./extensible'),
     Grammar    = require('./grammar'),
     Socket     = require('./socket');
 
 var Server = Class({ className: 'Server',
-  META_METHODS: ['handshake', 'connect', 'disconnect', 'subscribe', 'unsubscribe'],
-
   initialize: function(options) {
     this._options  = options || {};
     var engineOpts = this._options.engine || {};
     engineOpts.timeout = this._options.timeout;
     this._engine   = Engine.get(engineOpts);
@@ -118,14 +116,14 @@ var Server = Class({ className: 'Server',
     response.successful = !response.error;
     callback.call(context, [response]);
   },
 
   _handleMeta: function(message, local, callback, context) {
-    var method = Channel.parse(message.channel)[1],
+    var method = this._methodFor(message),
         response;
 
-    if (array.indexOf(this.META_METHODS, method) < 0) {
+    if (method === null) {
       response = this._makeResponse(message);
       response.error = Error.channelForbidden(message.channel);
       response.successful = false;
       return callback.call(context, [response]);
     }
@@ -135,10 +133,22 @@ var Server = Class({ className: 'Server',
       for (var i = 0, n = responses.length; i < n; i++) this._advize(responses[i], message.connectionType);
       callback.call(context, responses);
     }, this);
   },
 
+  _methodFor: function(message) {
+    var channel = message.channel;
+
+    if (channel === Channel.HANDSHAKE)   return 'handshake';
+    if (channel === Channel.CONNECT)     return 'connect';
+    if (channel === Channel.SUBSCRIBE)   return 'subscribe';
+    if (channel === Channel.UNSUBSCRIBE) return 'unsubscribe';
+    if (channel === Channel.DISCONNECT)  return 'disconnect';
+
+    return null;
+  },
+
   _advize: function(response, connectionType) {
     if (array.indexOf([Channel.HANDSHAKE, Channel.CONNECT], response.channel) < 0)
       return;
 
     var interval, timeout;

Vulnerable code:     META_METHODS = %w[handshake connect disconnect subscribe unsubscribe]

      method = Channel.parse(message['channel'])[1]
      unless META_METHODS.include?(method)
  META_METHODS: ['handshake', 'connect', 'disconnect', 'subscribe', 'unsubscribe'],

    var method = Channel.parse(message.channel)[1],
    if (array.indexOf(this.META_METHODS, method) < 0) {
Fixed code:       method = method_for(message)
      unless method
    def method_for(message)
      case message['channel']
      when Channel::HANDSHAKE   then :handshake
      when Channel::CONNECT     then :connect
      when Channel::SUBSCRIBE   then :subscribe
      when Channel::UNSUBSCRIBE then :unsubscribe
      when Channel::DISCONNECT  then :disconnect
      end
    end

  describe("with subscription auth installed", function() { with(this) {
    before(function() { with(this) {
      var extension = {
        incoming: function(message, callback) {
          if (message.channel === "/meta/subscribe" && !message.auth) {
            message.error = "Invalid auth"
          }
          callback(message)
        }
      }
      server.addExtension(extension)
    }})

    it("does not subscribe using the intended channel", function() { with(this) {
      var message = {
        channel: "/meta/subscribe",
        clientId: "fakeclientid",
        subscription: "/foo"
      }
      stub(engine, "clientExists").yields([true])
      expect(engine, "subscribe").exactly(0)
      server.process(message, false, function() {})
    }})

    it("does not subscribe using an extended channel", function() { with(this) {
      var message = {
        channel: "/meta/subscribe/x",
        clientId: "fakeclientid",
        subscription: "/foo"
      }
      stub(engine, "clientExists").yields([true])
      expect(engine, "subscribe").exactly(0)
      server.process(message, false, function() {})
    }})
  }})

  describe "with subscription auth installed" do
    before do
      extension = Class.new do
        def incoming(message, callback)
          if message["channel"] == "/meta/subscribe" and !message["auth"]
            message["error"] = "Invalid auth"
          end
          callback.call(message)
        end
      end
      server.add_extension(extension.new)
    end

    it "does not subscribe using the intended channel" do
      message = {
        "channel" => "/meta/subscribe",
        "clientId" => "fakeclientid",
        "subscription" => "/foo"
      }
      engine.stub(:client_exists).and_yield(true)
      engine.should_not_receive(:subscribe)
      server.process(message, false) {}
    end

    it "does not subscribe using an extended channel" do
      message = {
        "channel" => "/meta/subscribe/x",
        "clientId" => "fakeclientid",
        "subscription" => "/foo"
      }
      engine.stub(:client_exists).and_yield(true)
      engine.should_not_receive(:subscribe)
      server.process(message, false) {}
    end
  end

    var method = this._methodFor(message),
    if (method === null) {
  _methodFor: function(message) {
    var channel = message.channel;

    if (channel === Channel.HANDSHAKE)   return 'handshake';
    if (channel === Channel.CONNECT)     return 'connect';
    if (channel === Channel.SUBSCRIBE)   return 'subscribe';
    if (channel === Channel.UNSUBSCRIBE) return 'unsubscribe';
    if (channel === Channel.DISCONNECT)  return 'disconnect';

    return null;
  },