From 4bd8412633be654e0000874109250d57f2081db8 Mon Sep 17 00:00:00 2001 From: Forrest Fleming Date: Thu, 11 May 2017 19:37:23 -0700 Subject: [PATCH 01/10] Add login fields to camels --- app/models/camel.rb | 5 ++++ ...170512021209_add_login_fields_to_camels.rb | 7 +++++ db/schema.rb | 16 +++++++---- db/seeds.rb | 27 ++++++++++++++++--- 4 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20170512021209_add_login_fields_to_camels.rb diff --git a/app/models/camel.rb b/app/models/camel.rb index 91aa43a..84f02ab 100644 --- a/app/models/camel.rb +++ b/app/models/camel.rb @@ -1,2 +1,7 @@ class Camel < ApplicationRecord + validates :name, presence: true + validates :breed, presence: true + validates :email, presence: true + validates_uniqueness_of :email + validates :password, presence: true end diff --git a/db/migrate/20170512021209_add_login_fields_to_camels.rb b/db/migrate/20170512021209_add_login_fields_to_camels.rb new file mode 100644 index 0000000..cd9a2fa --- /dev/null +++ b/db/migrate/20170512021209_add_login_fields_to_camels.rb @@ -0,0 +1,7 @@ +class AddLoginFieldsToCamels < ActiveRecord::Migration[5.0] + def change + add_column :camels, :email, :string, unique: true + add_column :camels, :password, :string + add_column :camels, :is_admin, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 44f2a65..4286e11 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,14 +10,20 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161224052218) do +ActiveRecord::Schema.define(version: 20170512021209) do + + # These are extensions that must be enabled in order to support this database + enable_extension "plpgsql" create_table "camels", force: :cascade do |t| - t.string "name", null: false - t.string "breed", null: false + t.string "name", null: false + t.string "breed", null: false t.string "notes" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.string "email" + t.string "password" + t.boolean "is_admin", default: false end end diff --git a/db/seeds.rb b/db/seeds.rb index ec199b7..d0429e9 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1,3 +1,24 @@ -Camel.create!(name: "Perl", breed: "dromedary", notes: "Enjoys regex") -Camel.create!(name: "Joe", breed: "dromedary", notes: "Particularly cool") -Camel.create!(name: "Alphons", breed: "bactrian", notes: "Has pretty eyes") +Camel.create!(name: "Perl", + breed: "dromedary", + notes: "Enjoys regex", + email: "perl@perl.org", + password: "3usq7#bJCn4E", + is_admin: true) +Camel.create!(name: "Joe", + breed: "dromedary", + notes: "Particularly cool", + email: "joe@camel.com", + password: "love", + is_admin: false) +Camel.create!(name: "Alphons", + breed: "bactrian", + notes: "Has pretty eyes", + email: "alphons@perl.net", + password: "zKQ4xFZn!7g%", + is_admin: false) +Camel.create!(name: "Charles", + breed: "ACTUALLY A POODLE", + notes: "How did a dog get in the camels table computers are hard", + email: "charles@poodleparade.com", + password: "i<3p0rkC#opz", + is_admin: false) From 1464c193f1f076770871d7678954eada53c1c525 Mon Sep 17 00:00:00 2001 From: Forrest Fleming Date: Thu, 11 May 2017 19:37:55 -0700 Subject: [PATCH 02/10] use db for conditional hashing --- app/services/conditional_hashing.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/services/conditional_hashing.rb b/app/services/conditional_hashing.rb index ddf1c31..35943eb 100644 --- a/app/services/conditional_hashing.rb +++ b/app/services/conditional_hashing.rb @@ -8,7 +8,7 @@ def initialize(login, delta: max_delta) # Return value is whether @login is valid def execute - found = valid_logins.include?(login) + found = Camel.where(email: login).present? sleep(delta) if found found end @@ -22,13 +22,8 @@ def hint attr_reader :login, :delta MAX_DELTA = 0.10 - LOGINS = %w(charles@poodles.com camel@sahara.com bactrian@dev.null dromedary@dev.null).freeze def max_delta MAX_DELTA end - - def valid_logins - LOGINS - end end From 5b5cf5569b86de4f0a92a77506d55c56bbb9cedd Mon Sep 17 00:00:00 2001 From: Forrest Fleming Date: Thu, 11 May 2017 19:38:46 -0700 Subject: [PATCH 03/10] inject comparator dependency ensure non-nil --- app/services/insecure_string_comparison.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/services/insecure_string_comparison.rb b/app/services/insecure_string_comparison.rb index 3c8afd5..f29a581 100644 --- a/app/services/insecure_string_comparison.rb +++ b/app/services/insecure_string_comparison.rb @@ -1,9 +1,10 @@ # Performs insecure string comparison, wherein the first non-matching byte # causes an early return. class InsecureStringComparison - def initialize(candidate, delta: max_delta) + def initialize(candidate, delta: max_delta, against: BITSTREAM) @delta = [max_delta, delta.to_f].min @candidate = candidate.to_s.bytes + @bitstream = against.to_s end def execute @@ -39,12 +40,8 @@ def hint MAX_DELTA = 0.05 - def bitstream - BITSTREAM - end - def max_delta MAX_DELTA end - attr_reader :candidate, :delta + attr_reader :candidate, :delta, :bitstream end From 7c59e8c25e4318dcf7636516067bacdc8ada40ea Mon Sep 17 00:00:00 2001 From: Forrest Fleming Date: Thu, 11 May 2017 20:24:15 -0700 Subject: [PATCH 04/10] prefer :email param in public/index, too --- README.md | 8 ++++---- app/controllers/timing_vulnerabilities_controller.rb | 4 ++-- public/index.html | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 005ac87..5bd8797 100644 --- a/README.md +++ b/README.md @@ -6,9 +6,9 @@ vulnerable and configurable in its vulnerability. Route | Required parameters | Optional Parameters --- | --- | --- -`/timing/conditional_hashing` | `login` | `delta` +`/timing/conditional_hashing` | `email` | `delta` `/timing/string_comparison` | `password` | `delta` -`/timing/login` | `login`, `password` | `delta` +`/timing/login` | `email`, `password` | `delta` `/timing/basic_auth` | `password` | `delta` `/injections/sql/raw_sql` | `name` | `/injections/sql/raw_where` | `name` | @@ -27,7 +27,7 @@ Edit `.env` to contain appropriate values, then ## Timing Vulnerabilities ### Conditional Hashing -The `login` parameter is the email address to test for inclusion in the +The `email` parameter is the email address to test for inclusion in the "database," and is required. The `delta` URL parameter can be provided to tell the application the timing @@ -48,7 +48,7 @@ comparison (that is, no additional delay is introduced). ### Login -`login` and `password` parameters are required. +`email` and `password` parameters are required. This endpoint simply combines conditional hashing and insecure string comparison for a full vulnerable login experience. A provided `delta` parameter will be diff --git a/app/controllers/timing_vulnerabilities_controller.rb b/app/controllers/timing_vulnerabilities_controller.rb index 544af0b..a80f63f 100644 --- a/app/controllers/timing_vulnerabilities_controller.rb +++ b/app/controllers/timing_vulnerabilities_controller.rb @@ -30,7 +30,7 @@ def index private def user_finder - @user_finder ||= ConditionalHashing.new(timing_params[:login], + @user_finder ||= ConditionalHashing.new(timing_params[:email], delta: timing_params[:delta]) end @@ -40,7 +40,7 @@ def string_comparator end def timing_params - params.permit(:login, :password, :delta) + params.permit(:email, :password, :delta) end def determine_access(bool) diff --git a/public/index.html b/public/index.html index 3554f03..1063d75 100644 --- a/public/index.html +++ b/public/index.html @@ -12,7 +12,7 @@

Camelflage

/timing/conditional_hashing - login + email delta @@ -22,7 +22,7 @@

Camelflage

/timing/login - login, password + email, password delta @@ -56,7 +56,7 @@

Timing Vulnerabilities

Conditional Hashing

-

The login parameter is the email address to test for inclusion in the +

The email parameter is the email address to test for inclusion in the "database," and is required.

The delta URL parameter can be provided to tell the application the timing @@ -77,7 +77,7 @@

Insecure string comparison

Login

-

login and password parameters are required.

+

email and password parameters are required.

This endpoint simply combines conditional hashing and insecure string comparison for a full vulnerable login experience. A provided delta parameter will be From 20b6f228f8156ac290d873b927ca1d3a1a233d10 Mon Sep 17 00:00:00 2001 From: Forrest Fleming Date: Thu, 11 May 2017 20:26:27 -0700 Subject: [PATCH 05/10] allow session manipulation --- app/controllers/timing_vulnerabilities_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/timing_vulnerabilities_controller.rb b/app/controllers/timing_vulnerabilities_controller.rb index a80f63f..9f9d25a 100644 --- a/app/controllers/timing_vulnerabilities_controller.rb +++ b/app/controllers/timing_vulnerabilities_controller.rb @@ -43,8 +43,9 @@ def timing_params params.permit(:email, :password, :delta) end - def determine_access(bool) + def determine_access(bool, set: nil) if bool + session[:email] = set if set render plain: "Authorized", status: :ok else render plain: "Unauthorized", status: :unauthorized From 195a76f17abaff1e765bdb51b94c40bdb790ef8f Mon Sep 17 00:00:00 2001 From: Forrest Fleming Date: Thu, 11 May 2017 20:27:06 -0700 Subject: [PATCH 06/10] compare login flow against db values --- app/controllers/timing_vulnerabilities_controller.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/controllers/timing_vulnerabilities_controller.rb b/app/controllers/timing_vulnerabilities_controller.rb index 9f9d25a..4ebf6ae 100644 --- a/app/controllers/timing_vulnerabilities_controller.rb +++ b/app/controllers/timing_vulnerabilities_controller.rb @@ -1,8 +1,13 @@ class TimingVulnerabilitiesController < ApplicationController def login - user_result = user_finder.execute - comparator_result = string_comparator.execute - determine_access(user_result && comparator_result) + user_result = ConditionalHashing.new(timing_params[:email], + delta: timing_params[:delta] + ).execute + actual_password = Camel.where(email: timing_params[:email]).pluck(:password).take + comparator_result = InsecureStringComparison.new(timing_params[:password], + against: actual_password + ).execute + determine_access(user_result && comparator_result, set: timing_params[:email]) end def conditional_hashing From f0ca384a26a19f1e2bbe6cc0035313dab19ebf90 Mon Sep 17 00:00:00 2001 From: Forrest Fleming Date: Thu, 11 May 2017 20:27:20 -0700 Subject: [PATCH 07/10] route for /login --- config/routes.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/routes.rb b/config/routes.rb index 98fdf3e..18e916d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,4 +1,6 @@ Rails.application.routes.draw do + post :login, to: 'timing_vulnerabilities#login' + resource :timing, controller: 'timing_vulnerabilities', only: [] do get :index match :login, via: %i(post get) From d2d56037544541096f70e88f55831f4a532ca69a Mon Sep 17 00:00:00 2001 From: Forrest Fleming Date: Thu, 11 May 2017 20:30:27 -0700 Subject: [PATCH 08/10] use proper method .take witout args only works on AR collections, but pluck returns an Array (and so take requires an arg). Prefer first, whose implementation is consistent across the two classes --- app/controllers/timing_vulnerabilities_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/timing_vulnerabilities_controller.rb b/app/controllers/timing_vulnerabilities_controller.rb index 4ebf6ae..ee59df4 100644 --- a/app/controllers/timing_vulnerabilities_controller.rb +++ b/app/controllers/timing_vulnerabilities_controller.rb @@ -3,7 +3,7 @@ def login user_result = ConditionalHashing.new(timing_params[:email], delta: timing_params[:delta] ).execute - actual_password = Camel.where(email: timing_params[:email]).pluck(:password).take + actual_password = Camel.where(email: timing_params[:email]).pluck(:password).first comparator_result = InsecureStringComparison.new(timing_params[:password], against: actual_password ).execute From 51943885bb7cca87435dade709a7595924b64cb2 Mon Sep 17 00:00:00 2001 From: Forrest Fleming Date: Thu, 11 May 2017 21:18:53 -0700 Subject: [PATCH 09/10] prefer html response for template injection --- app/controllers/injections/template_injections_controller.rb | 2 +- app/services/template_injection.rb | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/injections/template_injections_controller.rb b/app/controllers/injections/template_injections_controller.rb index 9259845..f8adef1 100644 --- a/app/controllers/injections/template_injections_controller.rb +++ b/app/controllers/injections/template_injections_controller.rb @@ -7,7 +7,7 @@ def index end def interpolation - render plain: injection_command.execute + render html: injection_command.execute end private diff --git a/app/services/template_injection.rb b/app/services/template_injection.rb index 7e2f633..df98248 100644 --- a/app/services/template_injection.rb +++ b/app/services/template_injection.rb @@ -8,13 +8,14 @@ def hint end def execute - erb.result + erb.result.html_safe end private def interpolate(string) <<-EOF.strip_heredoc + 

     +--------------------------------------+
     |                Hello, #{string}             |
     +--------------------------------------+
@@ -24,6 +25,7 @@ def interpolate(string)
     `-,,,  ,_      ;'~U'
       _,-' ,'`-__; '--.
     (_/'~~      ''''(;
+
EOF end From f9ded46804748cd3d4ca5fa30ee0f20e8b340987 Mon Sep 17 00:00:00 2001 From: Forrest Fleming Date: Thu, 11 May 2017 21:19:33 -0700 Subject: [PATCH 10/10] Turn off XSS protection in browser --- config/application.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 2bfb086..fe9049a 100644 --- a/config/application.rb +++ b/config/application.rb @@ -8,6 +8,10 @@ module Camelflage class Application < Rails::Application - # config.api_only = true + config.action_dispatch.default_headers = { + 'X-Frame-Options' => 'SAMEORIGIN', + 'X-XSS-Protection' => '0', + 'X-Content-Type-Options' => 'nosniff' + } end end