Today’s Email Incident

Earlier today a routine system email was incorrectly sent to many of our GitHub Enterprise customers.
In these errant emails, customer email addresses were included in the To: field, making them visible to anyone who received the message.

We are very sorry about this. We have determined what caused this incident and contacted all affected customers directly.

Background

The incident occured in the Rails application we use to manage trials and customer contact information for
GitHub Enterprise, not the product itself. No GitHub Enterprise installations were affected, and no license keys
or any other data were exposed. GitHub.com was not affected.

As part of a routine daily process, the system notifies the members of any organization whose
license is about to expire about the upcoming need for renewal. The app builds an email message
including the addresses of all of the active accounts tied to the given organization, putting them
in the To: field to enhance deliverability. This morning, the email included
a great many more addresses than expected.

Technical details

Yesterday the Rails core team released four security patches
(CVE-2013-1854, CVE-2013-1855, CVE-2013-1856, CVE-2013-1857).
We immediately reviewed the patches and updated our Rails applications to stay current.
Unfortunately one of these security patches included a change that caused certain SQL queries to behave unexpectedly.

Here’s an example of this change in behavior:

class Organization < ActiveRecord::Base
  has_many :teams

  attr_accessible :name, :has_octocats

  scope :has_octocats_scope, lambda { where(:has_octocats => true) }

  def self.has_octocats_class_method
    where(:has_octocats => true)
  end
end

class Team < ActiveRecord::Base
  belongs_to :organization
  attr_accessible :name

  def self.using_octocats_scope
    where(:organization_id => Organization.has_octocats_scope.select(:id))
  end

  def self.using_octocats_class_method
    where(:organization_id => Organization.has_octocats_class_method.select(:id))
  end
end

> github = Organization.create(:name => "GitHub", :has_octocats => true)
> acme   = Organization.create(:name => "Acme",   :has_octocats => false)
> github.teams.create(:name => "Supportocats")
> acme.teams.create(:name => "Roadrunners")
> github.id
#=> 1
> acme.id
#=> 2

So, an Organization owns a number of Team records. We’ve defined a couple of methods to
help us scope queries for teams to only those organizations that have octocats.
Ideally, both of these methods will scope to the same thing: only Team records with an
organization_id of 1, the GitHub Organization. And prior to this latest Rails release, they did.

But the latest release of Rails introduced a subtle change to this behavior. Let’s try to make some queries
based on the Organization‘s teams:

> teams = github.teams
  Team Load (0.4ms)  SELECT `teams`.* FROM `teams` WHERE `teams`.`organization_id` = 1
> teams.length       # => 1
> teams.first.name   # => "Supportocats"

Great. Here we’ve asked for the GitHub organization’s teams, and we’ve gotten the correct one,
“Supportocats”, back. All is good so far. Now let’s use one of our scopes, just to be extra specific:

> teams = github.teams.using_octocats_class_method
  Team Load (0.4ms)  SELECT `teams`.* FROM `teams` WHERE `teams`.`organization_id` = 1 AND `teams`.`organization_id` IN (1)
> teams.length       # => 1
> teams.first.name   # => "Supportocats"

The results are the same, but the query is different. By going through an extra scope, we’ve added an additional
SQL predicate, one that says the returned Team records must belong to an Organization that has octocats. Since
the GitHub team has them, the result is the same.

Let’s try our scope that is restricted to octocat-having teams on the Acme org:

> teams = acme.teams.using_octocats_class_method
  Team Load (0.4ms)  SELECT `teams`.* FROM `teams` WHERE `teams`.`organization_id` = 2 AND `teams`.`organization_id` IN (1)
> teams.length   # => 0

Here we see a different result, as expected, and a similar query, again asking for all of the Acme organization’s teams
that also belong to an Organization that has octocats. The Acme Organization has none, so no teams are returned.

But now we come to an unexpected difference. In the last couple of examples, we were using an Arel scope on Organization that
was defined as a normal class method. But if we change to using the scope defined with ActiveRecord’s scope method,
we get unexpected and potentially dangerous results:

> teams = acme.teams.using_octocats_scope
  Team Load (0.4ms)  SELECT `teams`.* FROM `teams` WHERE `teams`.`organization_id` IN (1)
> teams.length       # => 1
> teams.first.name   # => "Supportocats"

Now the Acme organization is returning the GitHub organization’s teams! This is obviously bad behavior. What’s happening? In
this case, when using the scope method to define an Arel scope on Organization, the where clause of the
scope is overriding the condition imposed by the Organization#teams association. The part of the WHERE clause
meant to restrict the query to Team records related to the Acme organization was dropped.

We’ve narrowed down this change in behavior to this commit.
We have fixed this issue on our affected applications and are working with the Rails core team to determine if this change was intentional as well as what action other users should take.

What we’re doing about it

We’re reviewing every piece of GitHub code that touches email so we can keep this from happening in the future.
We’re focusing on more stringent automated tests, sanity checks on email recipients, and even more careful review when we upgrade an external dependency like Rails.

Written by

Brian Doll

@briandoll