From d445e59a98aa3dc46954f46ea9bb26785b68a2bb Mon Sep 17 00:00:00 2001
From: Ken Johnson
def download
- begin
- file = Rails.root.join('public', 'docs', params[:name])
- send_file file, :disposition => 'attachment'
- rescue
- redirect_to user_benefit_forms_path(:user_id => current_user.user_id)
- end
- end
+ begin
+ path = Rails.root.join('public', 'docs', params[:name])
+ file = params[:type].constantize.new(path)
+ send_file file, :disposition => 'attachment'
+ rescue
+ redirect_to user_benefit_forms_path(:user_id => current_user.user_id)
+ end
+ end
- The location of the file to render is dynamically generated based on user input (params[:name]). This means the user controls the location of the file to be retrieved. + The location of the file to render is dynamically generated based on user input (params[:name]). This means the user controls the location of the file to be retrieved. Additionally, the params[:type] (File) is not validated to make sure it matches up with expected values.
@@ -85,15 +86,19 @@ In this instance and as always, there are multiple ways to fix this. A simple method to secure this function by validating user input is as follows:
- # More secure version
+ # More secure version
def download
- file_assoc = {"1" => "Health_n_Stuff.pdf", "2" => "Dental_n_Stuff.pdf"}
+ file_assoc = {"1" => "Health_n_Stuff.pdf", "2" => "Dental_n_Stuff.pdf"}
begin
- if file_assoc.has_key?(params[:name].to_s)
- file = Rails.root.join('public', 'docs', file_assoc[params[:name].to_s])
- send_file file, :disposition => 'attachment'
+ if file_assoc.has_key?(params[:name].to_s)
+ path = Rails.root.join('public', 'docs', file_assoc[params[:name].to_s])
+ if params[:type] == "File"
+ file = params[:type].constantize.new(path)
+ send_file file, :disposition => 'attachment'
+ end
else
file = Rails.root.join('public', 'docs', "Dental_n_Stuff.pdf")
+ send_file file, :disposition => 'attachment'
end
rescue
redirect_to user_benefit_forms_path(:user_id => current_user.user_id)