From 932d2304f9ebdc0c74ab0d3623ad213701d69bdb Mon Sep 17 00:00:00 2001 From: cktricky Date: Wed, 12 Mar 2014 12:38:41 -0400 Subject: [PATCH 1/6] okay first run at making an API for railsgoat --- app/controllers/api/v1/users_controller.rb | 11 +++++++++++ app/helpers/api/v1/users_helper.rb | 2 ++ config/routes.rb | 8 +++++++- spec/controllers/api/v1/users_controller_spec.rb | 5 +++++ spec/helpers/api/v1/users_helper_spec.rb | 15 +++++++++++++++ 5 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 app/controllers/api/v1/users_controller.rb create mode 100644 app/helpers/api/v1/users_helper.rb create mode 100644 spec/controllers/api/v1/users_controller_spec.rb create mode 100644 spec/helpers/api/v1/users_helper_spec.rb diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb new file mode 100644 index 0000000..2afc41e --- /dev/null +++ b/app/controllers/api/v1/users_controller.rb @@ -0,0 +1,11 @@ +class Api::V1::UsersController < ApplicationController + + skip_before_filter :authenticated + + respond_to :json + + def index + respond_with ({:hi => :world}) + end + +end diff --git a/app/helpers/api/v1/users_helper.rb b/app/helpers/api/v1/users_helper.rb new file mode 100644 index 0000000..4d5288c --- /dev/null +++ b/app/helpers/api/v1/users_helper.rb @@ -0,0 +1,2 @@ +module Api::V1::UsersHelper +end diff --git a/config/routes.rb b/config/routes.rb index 9c21e1a..2383276 100755 --- a/config/routes.rb +++ b/config/routes.rb @@ -33,7 +33,7 @@ Railsgoat::Application.routes.draw do resources :messages do end - + end get "download" => "benefit_forms#download" @@ -81,6 +81,12 @@ Railsgoat::Application.routes.draw do get "home" end end + + namespace :api, defaults: {format: 'json'} do + namespace :v1 do + resources :users + end + end root :to => "sessions#new" diff --git a/spec/controllers/api/v1/users_controller_spec.rb b/spec/controllers/api/v1/users_controller_spec.rb new file mode 100644 index 0000000..184b048 --- /dev/null +++ b/spec/controllers/api/v1/users_controller_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe Api::V1::UsersController do + +end diff --git a/spec/helpers/api/v1/users_helper_spec.rb b/spec/helpers/api/v1/users_helper_spec.rb new file mode 100644 index 0000000..13a6067 --- /dev/null +++ b/spec/helpers/api/v1/users_helper_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +# Specs in this file have access to a helper object that includes +# the Api::V1::UsersHelper. For example: +# +# describe Api::V1::UsersHelper do +# describe "string concat" do +# it "concats two strings with spaces" do +# expect(helper.concat_strings("this","that")).to eq("this that") +# end +# end +# end +describe Api::V1::UsersHelper do + pending "add some examples to (or delete) #{__FILE__}" +end From f4f5d5744cc064a3612d87bb885061d5a09d96f3 Mon Sep 17 00:00:00 2001 From: cktricky Date: Wed, 12 Mar 2014 13:24:37 -0400 Subject: [PATCH 2/6] working on the auth structure for the API --- app/controllers/api/v1/users_controller.rb | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index 2afc41e..5f59b33 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -1,11 +1,20 @@ class Api::V1::UsersController < ApplicationController skip_before_filter :authenticated - + before_filter :valid_api_token + respond_to :json - def index - respond_with ({:hi => :world}) + def valid_api_token + authenticate_or_request_with_http_token do |token, options| + # TODO :add some functionality to check if the HTTP Header is valid + return true + end end - + + def index + respond_with User.all + end + + end From 95eb5a56fdecc079c727affe14472fb7a0492727 Mon Sep 17 00:00:00 2001 From: cktricky Date: Wed, 12 Mar 2014 15:40:12 -0400 Subject: [PATCH 3/6] added vulnerable auth check for the API --- app/controllers/api/v1/users_controller.rb | 43 ++++++++++++++++++---- config/initializers/constants.rb | 1 + 2 files changed, 36 insertions(+), 8 deletions(-) create mode 100644 config/initializers/constants.rb diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index 5f59b33..a23daf3 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -2,19 +2,46 @@ class Api::V1::UsersController < ApplicationController skip_before_filter :authenticated before_filter :valid_api_token + before_filter :extrapolate_user respond_to :json - def valid_api_token - authenticate_or_request_with_http_token do |token, options| - # TODO :add some functionality to check if the HTTP Header is valid - return true - end - end - def index - respond_with User.all + respond_with @user end +private + + def valid_api_token + authenticate_or_request_with_http_token do |token, options| + # TODO :add some functionality to check if the HTTP Header is valid + identify_user(token) + end + end + + def identify_user(token="") + # We've had issues with URL encoding, etc. causing issues so just to be safe + # we will go ahead and unescape the user's token + unescape_token(token) + @clean_token =~ /(.*?)-(.*)/ + id = $1 + hash = $2 + (id && hash) ? true : false + check_hash(id, hash) ? true : false + end + + def check_hash(id, hash) + digest = OpenSSL::Digest::SHA1.hexdigest("#{ACCESS_TOKEN_SALT}:#{id}") + hash == digest + end + + def unescape_token(token="") + @clean_token = CGI::unescape(token) + end + + # Added a method to make it easy to figure out who the user is. + def extrapolate_user + @user = User.find_by_id(@clean_token.split("-").first) + end end diff --git a/config/initializers/constants.rb b/config/initializers/constants.rb new file mode 100644 index 0000000..7fdcd8f --- /dev/null +++ b/config/initializers/constants.rb @@ -0,0 +1 @@ +ACCESS_TOKEN_SALT = "S4828341189aefiasd#ASDF" \ No newline at end of file From 48ddc999557a8eda671d86c666c8ac5661d047fc Mon Sep 17 00:00:00 2001 From: cktricky Date: Wed, 12 Mar 2014 17:45:08 -0400 Subject: [PATCH 4/6] some basic api functionality with a few gotchas --- app/controllers/api/v1/users_controller.rb | 12 ++++++++++-- app/models/user.rb | 7 +++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index a23daf3..d995abb 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -5,9 +5,17 @@ class Api::V1::UsersController < ApplicationController before_filter :extrapolate_user respond_to :json - + def index - respond_with @user + # We removed the .as_json code from the model, just seemed like extra work. + # dunno, maybe useful at a later time? + #respond_with @user.admin ? User.all.as_json : @user.as_json + + respond_with @user.admin ? User.all : @user + end + + def show + respond_with @user.as_json end private diff --git a/app/models/user.rb b/app/models/user.rb index 2af7dc2..7705a10 100755 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -37,6 +37,13 @@ class User < ActiveRecord::Base def full_name "#{self.first_name} #{self.last_name}" end + +=begin + # Instead of the entire user object being returned, we can use this to filter. + def as_json + super(only: [:user_id, :email, :first_name, :last_name]) + end +=end private From 4b0560a25003c6cd32b76c5981f92218f2f30e1f Mon Sep 17 00:00:00 2001 From: cktricky Date: Wed, 12 Mar 2014 18:59:38 -0400 Subject: [PATCH 5/6] whew, now THAT is a huge tutorial explanation for a relatively simple issue! --- app/controllers/api/v1/users_controller.rb | 2 + app/controllers/tutorials_controller.rb | 3 +- app/views/layouts/tutorial/_sidebar.html.erb | 3 + .../logic_flaws/_broken_regexp.html.erb | 219 ++++++++++++++++++ app/views/tutorials/logic_flaws.html.erb | 19 ++ config/routes.rb | 1 + 6 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 app/views/layouts/tutorial/logic_flaws/_broken_regexp.html.erb create mode 100644 app/views/tutorials/logic_flaws.html.erb diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index d995abb..6866774 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -43,6 +43,8 @@ private hash == digest end + # We had some issues with the token and url encoding... + # this is an attempt to normalize the data. def unescape_token(token="") @clean_token = CGI::unescape(token) end diff --git a/app/controllers/tutorials_controller.rb b/app/controllers/tutorials_controller.rb index a8d151c..69df74f 100755 --- a/app/controllers/tutorials_controller.rb +++ b/app/controllers/tutorials_controller.rb @@ -83,7 +83,8 @@ class TutorialsController < ApplicationController def guard end - + def logic_flaws + end def mass_assignment end diff --git a/app/views/layouts/tutorial/_sidebar.html.erb b/app/views/layouts/tutorial/_sidebar.html.erb index b46902b..644b772 100755 --- a/app/views/layouts/tutorial/_sidebar.html.erb +++ b/app/views/layouts/tutorial/_sidebar.html.erb @@ -112,6 +112,9 @@
  • <%= link_to "Constantize", constantize_tutorials_path %>
    • +
  • + <%= link_to "Logic Flaws", logic_flaws_tutorials_path %> +
    • + + + + + + \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 2383276..6dfde00 100755 --- a/config/routes.rb +++ b/config/routes.rb @@ -58,6 +58,7 @@ Railsgoat::Application.routes.draw do get "mass_assignment" get "constantize" get "gauntlt" + get "logic_flaws" end end From e49b43f89996dcafa3f512bd97813379081e3e87 Mon Sep 17 00:00:00 2001 From: cktricky Date: Wed, 12 Mar 2014 20:28:59 -0400 Subject: [PATCH 6/6] added the verbose model attributes finding under the exposure section within the tutorials --- .../_model_attributes_exposure.html.erb | 144 ++++++++++++++++++ app/views/tutorials/exposure.html.erb | 6 + 2 files changed, 150 insertions(+) create mode 100644 app/views/layouts/tutorial/exposure/_model_attributes_exposure.html.erb diff --git a/app/views/layouts/tutorial/exposure/_model_attributes_exposure.html.erb b/app/views/layouts/tutorial/exposure/_model_attributes_exposure.html.erb new file mode 100644 index 0000000..a5d4dd0 --- /dev/null +++ b/app/views/layouts/tutorial/exposure/_model_attributes_exposure.html.erb @@ -0,0 +1,144 @@ +
    +
    +
    + A6 - Sensitive Data Exposure - Model Attributes Exposure +
    +
    +
    +
    +
    +
    + + + + Description + +
    +
    +
    +

    + The application's API returns a model object (user or users). Using respond_with, the API returns the full model object. It is simple but exposes information such as the user's password and other user attributes that you may wish to keep invisible. +

    +
    +
    +
    +
    +
    + + + + Bug + +
    +
    +
    +

    + Within app/controllers/api/v1/users_controller.rb: +

    + 
    +				 def index
+			       # We removed the .as_json code from the model, just seemed like extra work.
+			       # dunno, maybe useful at a later time?
+			       #respond_with @user.admin ? User.all.as_json : @user.as_json
+
+			       respond_with @user.admin ? User.all : @user
+			     end
+
+			     def show
+			       respond_with @user.as_json
+			     end
+
    +

    + The as_json method referenced in the comments section of the index action exists within the user model in order to override and safely protect our model from only rendering certain attributes. It is unused (commented out), app/models/user.rb: +

    + 
    	
+				  # Instead of the entire user object being returned, we can use this to filter.
+				  def as_json
+				    super(only: [:user_id, :email, :first_name, :last_name])
+				  end
+
    +

    + When utilizing the method that most tutorials describe or advocate when rendering model objects via JSON in an API (unsafe), the response looks like this: +

    + 
    +	HTTP/1.1 200 OK
+	Content-Type: application/json; charset=utf-8
+	X-UA-Compatible: IE=Edge
+	ETag: "6b4caf343a20865de174b2b530b945dd"
+	Cache-Control: max-age=0, private, must-revalidate
+	X-Request-Id: c3b0a57861087c0b827aab231747ef0c
+	X-Runtime: 0.051734
+	Connection: close
+	
+	{"admin":false,"created_at":"2014-01-23T16:17:10Z","email":
+	"jack@metacorp.com","first_name":"Jack","id":2,"last_name":"Mannino","password":
+	"b46dd2888a0904972649cc880a93f4dd","updated_at":"2014-01-23T16:17:10Z","user_id":2}
+
    +

    + Note that all attributes associated with this user are returned via the API. +

    +
    +
    +
    +
    +
    + + + + Solution + +
    +
    +
    +

    Model Attributes Exposure - ATTACK

    +

    Use the API and review the data returned. Additional information on exploiting the API available under the Extras > Logic Flaws Section.

    +

    Model Attributes Exposure - SOLUTION

    +

    + Uncomment the as_json method within the user model. Additionally, call .as_json on any User model object you would like to return via the API or other means. Example: +

    + 
    +					respond_with @user.admin ? User.all.as_json : @user.as_json
+
    +

    + Upon uncommenting the as_json method within the User model, the as_json method will ensure the API output only returns those attributes you have allowed in the following code: +

    + 
    +				def as_json
+				  super(only: [:user_id, :email, :first_name, :last_name])
+				end
+
    +

    + The response from the API should look like: +

    + 
    +	HTTP/1.1 200 OK
+	Content-Type: application/json; charset=utf-8
+	X-UA-Compatible: IE=Edge
+	ETag: "2333488e856669ac637e37cb4cf09cb6"
+	Cache-Control: max-age=0, private, must-revalidate
+	X-Request-Id: baa6a1c90004838793614e4c61633767
+	X-Runtime: 0.092768
+	Connection: close
+
+	{"email":"jack@metacorp.com","first_name":"Jack","last_name":"Mannino","user_id":2}
+
    +
    +
    +
    +
    +
    + + + + Hint + +
    +
    +
    + We have an API available... what does it return? +
    +
    +
    +
    +
    +
    \ No newline at end of file diff --git a/app/views/tutorials/exposure.html.erb b/app/views/tutorials/exposure.html.erb index 3e99af5..1d92130 100755 --- a/app/views/tutorials/exposure.html.erb +++ b/app/views/tutorials/exposure.html.erb @@ -11,6 +11,12 @@ <%= render :partial => "layouts/tutorial/exposure/ssn" %> + +
    +
    + <%= render :partial => "layouts/tutorial/exposure/model_attributes_exposure" %> +
    +