Skip to content
Permalink
Browse files Browse the repository at this point in the history
CodeClimate-recommended security fixes
  • Loading branch information
Tim Morgan committed Jun 25, 2013
1 parent 00bfeb3 commit 6d667c1
Show file tree
Hide file tree
Showing 21 changed files with 57 additions and 28 deletions.
2 changes: 2 additions & 0 deletions Gemfile.d/development.rb
@@ -1,6 +1,8 @@
group :development do
gem 'yard', require: nil
gem 'redcarpet', require: nil, platform: :mri

gem 'json-schema', '< 2.0.0' # version 2.0 breaks fdoc
gem 'fdoc'
end

Expand Down
2 changes: 2 additions & 0 deletions Gemfile.d/utilities.rb
@@ -1,3 +1,5 @@
gem 'json'
gem 'git', github: 'RISCfuture/ruby-git'
gem 'user-agent'

gem 'safe_yaml'
17 changes: 10 additions & 7 deletions Gemfile.lock
Expand Up @@ -88,15 +88,15 @@ GEM
thor
find_or_create_on_scopes (1.3.0)
activerecord (>= 3.1)
font-awesome-rails (3.1.1.2)
font-awesome-rails (3.2.1.1)
railties (>= 3.2, < 5.0)
hike (1.2.2)
hike (1.2.3)
httpi (2.0.2)
rack
i18n (0.6.1)
jdbc-postgres (9.2.1002.1)
journey (1.0.4)
jquery-rails (3.0.0)
jquery-rails (3.0.1)
railties (>= 3.0, < 5.0)
thor (>= 0.14, < 2.0)
json (1.8.0)
Expand All @@ -119,7 +119,7 @@ GEM
mime-types (~> 1.16)
treetop (~> 1.4.8)
mime-types (1.23)
multi_json (1.7.5)
multi_json (1.7.7)
pg (0.15.1)
plist (3.1.0)
polyglot (0.3.3)
Expand Down Expand Up @@ -147,7 +147,7 @@ GEM
rake (>= 0.8.7)
rdoc (~> 3.4)
thor (>= 0.14.6, < 2.0)
rake (10.0.4)
rake (10.1.0)
rdoc (3.12.2)
json (~> 1.4)
redcarpet (2.3.0)
Expand All @@ -163,6 +163,7 @@ GEM
rspec-core (~> 2.13.0)
rspec-expectations (~> 2.13.0)
rspec-mocks (~> 2.13.0)
safe_yaml (0.9.3)
sass (3.2.9)
sass-rails (3.2.6)
railties (~> 3.2.0)
Expand Down Expand Up @@ -198,7 +199,7 @@ GEM
json
squash_uploader (1.0.1)
json
stringex (2.0.2)
stringex (2.0.3)
therubyracer (0.11.4)
libv8 (~> 3.11.8.12)
ref
Expand All @@ -208,7 +209,7 @@ GEM
thor (0.18.1)
tilt (1.4.1)
timeliness (0.3.7)
treetop (1.4.12)
treetop (1.4.14)
polyglot
polyglot (>= 0.3.1)
tzinfo (0.3.37)
Expand Down Expand Up @@ -246,6 +247,7 @@ DEPENDENCIES
has_metadata_column!
jquery-rails
json
json-schema (< 2.0.0)
json_serialize
kramdown
less-rails
Expand All @@ -255,6 +257,7 @@ DEPENDENCIES
rails (>= 3.2.0)
redcarpet
rspec-rails
safe_yaml
sass-rails
slugalicious
sql_origin
Expand Down
1 change: 0 additions & 1 deletion app/assets/javascripts/application.js
Expand Up @@ -26,7 +26,6 @@
//
//= require jquery
//= require jquery_ujs
//= require jquery-ui
//
//= require jquery-cookie
//= require jquery-leanModal
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v1_controller.rb
Expand Up @@ -121,7 +121,7 @@ def symbolication
def deobfuscation
require_params :api_key, :environment, :build, :namespace

map = YAML.load(Zlib::Inflate.inflate(Base64.decode64(params['namespace'])))
map = YAML.load(Zlib::Inflate.inflate(Base64.decode64(params['namespace'])), safe: true, deserialize_symbols: false)
return head(:unprocessable_entity) unless map.kind_of?(Squash::Java::Namespace)

deploy = Project.find_by_api_key!(params['api_key']).
Expand All @@ -145,7 +145,7 @@ def deobfuscation
def sourcemap
require_params :api_key, :environment, :revision, :sourcemap

sourcemap = YAML.load(Zlib::Inflate.inflate(Base64.decode64(params['sourcemap'])))
sourcemap = YAML.load(Zlib::Inflate.inflate(Base64.decode64(params['sourcemap'])), safe: true, deserialize_symbols: false)
return head(:unprocessable_entity) unless sourcemap.kind_of?(Squash::Javascript::SourceMap)

Project.find_by_api_key!(params['api_key']).
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/bugs_controller.rb
Expand Up @@ -115,7 +115,7 @@ def index
sort_column, default_dir = SORTS[params[:sort]]

dir = if params[:dir].kind_of?(String) then
SORT_DIRECTIONS.include?(params[:dir].upcase) ? params[:dir] : default_dir
SORT_DIRECTIONS.include?(params[:dir].upcase) ? params[:dir].upcase : default_dir
else
default_dir
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/occurrences_controller.rb
Expand Up @@ -88,7 +88,7 @@ def index
respond_to do |format|
format.json do
dir = params[:dir]
dir = 'desc' unless SORT_DIRECTIONS.include?(dir.try(:upcase))
dir = 'DESC' unless SORT_DIRECTIONS.include?(dir.try(:upcase))

@occurrences = @bug.occurrences.order("occurred_at #{dir}").limit(50)

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/sessions_controller.rb
Expand Up @@ -16,7 +16,7 @@
# {AuthenticationHelpers} for more information on how authentication works.

class SessionsController < ApplicationController
skip_before_filter :login_required, except: :destroy
skip_before_filter :login_required, only: [:new, :create]
before_filter :must_be_unauthenticated, except: :destroy

respond_to :html
Expand Down
5 changes: 5 additions & 0 deletions app/models/additions/ldap_authentication.rb
Expand Up @@ -16,6 +16,11 @@
# Squash install is configured to use LDAP-based authentication.

module LdapAuthentication
extend ActiveSupport::Concern

included do
attr_accessible :first_name, :last_name, as: :system
end

# @return [String] This user's LDAP distinguished name (DN).

Expand Down
2 changes: 1 addition & 1 deletion app/models/blame.rb
Expand Up @@ -30,7 +30,7 @@ class Blame < ActiveRecord::Base
validates :repository_hash, :revision, :blamed_revision,
presence: true,
length: {is: 40},
format: {with: /[0-9a-f]+/}
format: {with: /\A[0-9a-f]+\z/}
validates :file,
presence: true,
length: {maximum: 255}
Expand Down
4 changes: 2 additions & 2 deletions app/models/bug.rb
Expand Up @@ -159,12 +159,12 @@ class Bug < ActiveRecord::Base
numericality: {only_integer: true, greater_than: 0}
validates :blamed_revision, :resolution_revision,
length: {is: 40},
format: {with: /[0-9a-f]+/},
format: {with: /\A[0-9a-f]+\z/},
allow_nil: true
validates :revision,
presence: true,
length: {is: 40},
format: {with: /[0-9a-f]+/}
format: {with: /\A[0-9a-f]+\z/}
#validates :first_occurrence,
# presence: true
#validates :latest_occurrence,
Expand Down
2 changes: 1 addition & 1 deletion app/models/environment.rb
Expand Up @@ -66,7 +66,7 @@ class Environment < ActiveRecord::Base
presence: true,
length: {maximum: 100},
uniqueness: {case_sensitive: false, scope: :project_id},
format: {with: /[a-z0-9\-_]+/}
format: {with: /\A[a-z0-9\-_]+\z/}

scope :with_name, ->(name) { where("LOWER(name) = ?", name.downcase) }

Expand Down
1 change: 1 addition & 0 deletions app/models/event.rb
Expand Up @@ -44,6 +44,7 @@ class Event < ActiveRecord::Base
belongs_to :user, inverse_of: :events
has_many :user_events, dependent: :delete_all, inverse_of: :event

attr_accessible :bug, :user, :kind, :data
attr_readonly :bug, :user, :kind, :data

include JsonSerialize
Expand Down
7 changes: 6 additions & 1 deletion app/models/observers/comment_observer.rb
Expand Up @@ -30,7 +30,12 @@ def after_commit_on_create(comment)
private

def create_event(comment)
Event.create! bug_id: comment.bug_id, kind: 'comment', data: {'comment_id' => comment.id}, user_id: comment.user_id
Event.create! do |event|
event.bug_id = comment.bug_id
event.kind = 'comment'
event.data = {'comment_id' => comment.id}
event.user_id = comment.user_id
end
end

def watch_bug(comment)
Expand Down
12 changes: 7 additions & 5 deletions app/models/occurrence.rb
Expand Up @@ -378,8 +378,6 @@ class Occurrence < ActiveRecord::Base
belongs_to :redirect_target, class_name: 'Occurrence', foreign_key: 'redirect_target_id', inverse_of: :redirected_occurrence
has_one :redirected_occurrence, class_name: 'Occurrence', foreign_key: 'redirect_target_id', inverse_of: :redirect_target, dependent: :destroy

attr_readonly :bug, :revision, :number, :occurred_at

include HasMetadataColumn
has_metadata_column(
# Universal
Expand Down Expand Up @@ -466,6 +464,9 @@ class Occurrence < ActiveRecord::Base
color_depth: {type: Integer, allow_nil: true}
)

attr_accessible :revision, :occurred_at, :client, :crashed, *metadata_column_fields.keys
attr_readonly :bug, :revision, :number, :occurred_at

# Fields that cannot be used by the aggregation view. These are fields with a
# a continuous range of possible values, or fields with unusual data types.
NON_AGGREGATING_FIELDS = %w( number message backtraces ivars arguments env_vars
Expand All @@ -482,7 +483,7 @@ class Occurrence < ActiveRecord::Base
validates :revision,
presence: true,
length: {is: 40},
format: {with: /[0-9a-f]+/}
format: {with: /\A[0-9a-f]+\z/}
#validates :number,
# presence: true,
# numericality: {only_integer: true, greater_than: 0},
Expand Down Expand Up @@ -837,8 +838,9 @@ def recategorize!
blamer = Blamer.new(self)
new_bug = blamer.find_or_create_bug!
if new_bug.id != bug_id
copy = new_bug.occurrences.build
copy.attributes = attributes.except('number', 'id', 'bug_id')
copy = new_bug.occurrences.build
copy.assign_attributes attributes.except('number', 'id', 'bug_id'),
without_protection: true
copy.save!
blamer.reopen_bug_if_necessary! new_bug
redirect_to! copy
Expand Down
2 changes: 2 additions & 0 deletions app/models/slug.rb
Expand Up @@ -68,6 +68,8 @@ class Slug < ActiveRecord::Base
after_destroy :invalidate_cache
set_nil_if_blank :scope

attr_accessible :sluggable, :slug, :active, :scope

# Marks a slug as active and deactivates all other slugs assigned to the
# record.

Expand Down
2 changes: 1 addition & 1 deletion app/models/symbolication.rb
Expand Up @@ -63,7 +63,7 @@ class Symbolication < ActiveRecord::Base
validates :uuid,
presence: true,
uniqueness: true,
format: {with: /^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$/i}
format: {with: /\A[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}\z/i}

after_commit(on: :create) do |sym|
BackgroundRunner.run SymbolicationWorker, sym.id
Expand Down
2 changes: 1 addition & 1 deletion app/models/user.rb
Expand Up @@ -78,7 +78,7 @@ class User < ActiveRecord::Base
validates :username,
presence: true,
length: {maximum: 50},
format: {with: /[a-z0-9_\-]+/},
format: {with: /\A[a-z0-9_\-]+\z/},
uniqueness: true

before_validation(on: :create) { |obj| obj.username = obj.username.downcase if obj.username }
Expand Down
5 changes: 4 additions & 1 deletion config/application.rb
Expand Up @@ -27,6 +27,9 @@
# Bundler.require(:default, :assets, Rails.env)
end

# Load preinitializers
Dir.glob(File.expand_path('../preinitializers/**/*.rb', __FILE__)).each { |f| require f }

module Squash
class Application < Rails::Application
# Settings in config/environments/* take precedence over those specified here.
Expand Down Expand Up @@ -72,7 +75,7 @@ class Application < Rails::Application
# This will create an empty whitelist of attributes available for mass-assignment for all models
# in your app. As such, your models will need to explicitly whitelist or blacklist accessible
# parameters by using an attr_accessible or attr_protected declaration.
# config.active_record.whitelist_attributes = true
config.active_record.whitelist_attributes = true

# Enable the asset pipeline
config.assets.enabled = true
Expand Down
2 changes: 0 additions & 2 deletions config/environment.rb
Expand Up @@ -12,8 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

Dir.glob(File.expand_path('../preinitializers/**/*.rb', __FILE__)).each { |f| require f }

# Load the rails application
require File.expand_path('../application', __FILE__)

Expand Down
7 changes: 7 additions & 0 deletions config/preinitializers/safe_yaml.rb
@@ -0,0 +1,7 @@
# Let's make SafeYAML act exactly like YAML by default, to avoid surprising
# behavior.
SafeYAML::OPTIONS[:default_mode] = :unsafe
SafeYAML::OPTIONS[:deserialize_symbols] = true

# And let's whitelist the things we transmit over YAML
SafeYAML.whitelist! Squash::Javascript::SourceMap, Squash::Java::Namespace

0 comments on commit 6d667c1

Please sign in to comment.