Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Content Type Validation not failing for spoofed video file #2426

Closed
NeoElit opened this issue Apr 12, 2017 · 4 comments
Closed

Content Type Validation not failing for spoofed video file #2426

NeoElit opened this issue Apr 12, 2017 · 4 comments

Comments

@NeoElit
Copy link

NeoElit commented Apr 12, 2017

For the following model

class User < ActiveRecord::Base
  has_attached_file :snap, :styles => { :thumb => "100x100>" }, :default_url => "/images/:style/missing.png"
  validates_attachment_content_type :snap, :content_type => /\Avideo\/.*\Z/
end

on paperclip '4.3.1', rails '4.2.6', content spoof detector doesn't seem to work when simply renaming data3.pdf file to data3.mp4
According to docs, it should be working since 'file' command is identifying it properly.

file -b --mime /tmp/data3.mp4    =>>   application/pdf; charset=binary

Am I missing something here? Or is it a bug?

@NeoElit
Copy link
Author

NeoElit commented Apr 12, 2017

Dug a little deeper.
Seems like issue pops because
calculated_type_mismatch? fails since media_types_from_name contains higher level type "application" which is also returned by calculated_media_type, which means almost all contents other than the basic ones will be allowed.

For a pdf file with mp4 extension:
calculated_content_type => "application/pdf"
calculated_media_type => "application"
media_types_from_name => ["application", "audio", "video", "video"]

So I guess spoof detection will fail on all higher level content type checks.

@NeoElit
Copy link
Author

NeoElit commented Apr 12, 2017

Instead of

def calculated_media_type
  @calculated_media_type ||= calculated_content_type.split("/").first
end

how about matching with sub type for spoof detection?

def calculated_media_type
  @calculated_media_type ||= calculated_content_type.split("/").last.split(';').first
end

Not sure about it's side effects though.

@NeoElit
Copy link
Author

NeoElit commented Apr 21, 2017

Currently the following patch works for me:

module Paperclip
  class MediaTypeSpoofDetector

    def supplied_type_mismatch?
      supplied_media_type.present? && !media_types_from_name.include?(supplied_media_type)
    end

    def supplied_media_type
      @content_type.split("/").last
    end

    def media_types_from_name
      @media_types_from_name ||= content_types_from_name.collect(&:sub_type)
    end

    def calculated_media_type
      @calculated_media_type ||= calculated_content_type.split("/").last.split(';').first
    end

  end
end

@mike-burns
Copy link
Contributor

Looks like we found a workaround, closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants