Skip to content

Commit

Permalink
Prevent OpenURI redirection during API image upload
Browse files Browse the repository at this point in the history
Paperclip’s CVE-2017–0889 Server Side Request Forgery (SSRF)
vulnerability (https://medium.com/in-the-weeds/all-about-paperclips-cve-2017-0889-server-side-request-forgery-ssrf-vulnerability-8cb2b1c96fe8) was likely introduced in solidusio#3573.  This patch
ensures that redirection does not take place in the context of
uploading an image via the api.
  • Loading branch information
calebhaye committed Apr 22, 2020
1 parent 4a1824a commit 64228ac
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
2 changes: 1 addition & 1 deletion api/app/controllers/spree/api/images_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def scope
def prepared_attachment
uri = URI.parse image_params[:attachment]
if uri.is_a? URI::HTTP
URI.open(image_params[:attachment])
URI.open(image_params[:attachment], redirect: false)
else
image_params[:attachment]
end
Expand Down
17 changes: 15 additions & 2 deletions api/spec/requests/spree/api/images_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ module Spree
expect do
post spree.api_product_images_path(product.id), params: {
image: {
attachment: 'https://github.com/solidusio/brand/raw/1827e7afb7ebcf5a1fc9cf7bf6cf9d277183ef11/PNG/solidus-logo-dark.png',
attachment: 'https://raw.githubusercontent.com/solidusio/brand/1827e7afb7ebcf5a1fc9cf7bf6cf9d277183ef11/PNG/solidus-logo-dark.png',
viewable_type: 'Spree::Variant',
viewable_id: product.master.to_param,
alt: 'just a test'
Expand All @@ -48,6 +48,19 @@ module Spree
end.to change(Image, :count).by(1)
end

it 'will raise an exception if URL passed as attachment parameter attempts to redirect' do
expect do
post spree.api_product_images_path(product.id), params: {
image: {
attachment: 'https://github.com/solidusio/brand/raw/1827e7afb7ebcf5a1fc9cf7bf6cf9d277183ef11/PNG/solidus-logo-dark.png',
viewable_type: 'Spree::Variant',
viewable_id: product.master.to_param,
},
}
end.to raise_error(OpenURI::HTTPRedirect)
end


context "working with an existing product image" do
let!(:product_image) { product.master.images.create!(attachment: image('thinking-cat.jpg')) }

Expand Down Expand Up @@ -90,7 +103,7 @@ module Spree
put spree.api_variant_image_path(product.master.id, product_image), params: {
image: {
position: 2,
attachment: 'https://github.com/solidusio/brand/raw/1827e7afb7ebcf5a1fc9cf7bf6cf9d277183ef11/PNG/solidus-logo-dark.png'
attachment: 'https://raw.githubusercontent.com/solidusio/brand/1827e7afb7ebcf5a1fc9cf7bf6cf9d277183ef11/PNG/solidus-logo-dark.png'
},
}
expect(response.status).to eq(200)
Expand Down

0 comments on commit 64228ac

Please sign in to comment.