diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index 12e7f9f0b53c8..2295559572091 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -20,6 +20,301 @@ class ActiveStorage::Variation attr_reader :transformations + class UnsupportedImageProcessingMethod < StandardError; end + class UnsupportedImageProcessingArgument < StandardError; end + + SUPPORTED_IMAGE_PROCESSING_METHODS = [ + "adaptive_blur", + "adaptive_resize", + "adaptive_sharpen", + "adjoin", + "affine", + "alpha", + "annotate", + "antialias", + "append", + "apply", + "attenuate", + "authenticate", + "auto_gamma", + "auto_level", + "auto_orient", + "auto_threshold", + "backdrop", + "background", + "bench", + "bias", + "bilateral_blur", + "black_point_compensation", + "black_threshold", + "blend", + "blue_primary", + "blue_shift", + "blur", + "border", + "bordercolor", + "borderwidth", + "brightness_contrast", + "cache", + "canny", + "caption", + "channel", + "channel_fx", + "charcoal", + "chop", + "clahe", + "clamp", + "clip", + "clip_path", + "clone", + "clut", + "coalesce", + "colorize", + "colormap", + "color_matrix", + "colors", + "colorspace", + "colourspace", + "color_threshold", + "combine", + "combine_options", + "comment", + "compare", + "complex", + "compose", + "composite", + "compress", + "connected_components", + "contrast", + "contrast_stretch", + "convert", + "convolve", + "copy", + "crop", + "cycle", + "deconstruct", + "define", + "delay", + "delete", + "density", + "depth", + "descend", + "deskew", + "despeckle", + "direction", + "displace", + "dispose", + "dissimilarity_threshold", + "dissolve", + "distort", + "dither", + "draw", + "duplicate", + "edge", + "emboss", + "encoding", + "endian", + "enhance", + "equalize", + "evaluate", + "evaluate_sequence", + "extent", + "extract", + "family", + "features", + "fft", + "fill", + "filter", + "flatten", + "flip", + "floodfill", + "flop", + "font", + "foreground", + "format", + "frame", + "function", + "fuzz", + "fx", + "gamma", + "gaussian_blur", + "geometry", + "gravity", + "grayscale", + "green_primary", + "hald_clut", + "highlight_color", + "hough_lines", + "iconGeometry", + "iconic", + "identify", + "ift", + "illuminant", + "immutable", + "implode", + "insert", + "intensity", + "intent", + "interlace", + "interline_spacing", + "interpolate", + "interpolative_resize", + "interword_spacing", + "kerning", + "kmeans", + "kuwahara", + "label", + "lat", + "layers", + "level", + "level_colors", + "limit", + "limits", + "linear_stretch", + "linewidth", + "liquid_rescale", + "list", + "loader", + "log", + "loop", + "lowlight_color", + "magnify", + "map", + "mattecolor", + "median", + "mean_shift", + "metric", + "mode", + "modulate", + "moments", + "monitor", + "monochrome", + "morph", + "morphology", + "mosaic", + "motion_blur", + "name", + "negate", + "noise", + "normalize", + "opaque", + "ordered_dither", + "orient", + "page", + "paint", + "pause", + "perceptible", + "ping", + "pointsize", + "polaroid", + "poly", + "posterize", + "precision", + "preview", + "process", + "quality", + "quantize", + "quiet", + "radial_blur", + "raise", + "random_threshold", + "range_threshold", + "red_primary", + "regard_warnings", + "region", + "remote", + "render", + "repage", + "resample", + "resize", + "resize_to_fill", + "resize_to_fit", + "resize_to_limit", + "resize_and_pad", + "respect_parentheses", + "reverse", + "roll", + "rotate", + "sample", + "sampling_factor", + "saver", + "scale", + "scene", + "screen", + "seed", + "segment", + "selective_blur", + "separate", + "sepia_tone", + "shade", + "shadow", + "shared_memory", + "sharpen", + "shave", + "shear", + "sigmoidal_contrast", + "silent", + "similarity_threshold", + "size", + "sketch", + "smush", + "snaps", + "solarize", + "sort_pixels", + "sparse_color", + "splice", + "spread", + "statistic", + "stegano", + "stereo", + "storage_type", + "stretch", + "strip", + "stroke", + "strokewidth", + "style", + "subimage_search", + "swap", + "swirl", + "synchronize", + "taint", + "text_font", + "threshold", + "thumbnail", + "tile_offset", + "tint", + "title", + "transform", + "transparent", + "transparent_color", + "transpose", + "transverse", + "treedepth", + "trim", + "type", + "undercolor", + "unique_colors", + "units", + "unsharp", + "update", + "valid_image", + "view", + "vignette", + "virtual_pixel", + "visual", + "watermark", + "wave", + "wavelet_denoise", + "weight", + "white_balance", + "white_point", + "white_threshold", + "window", + "window_group", + ].concat(ActiveStorage.supported_image_processing_methods) + + UNSUPPORTED_IMAGE_PROCESSING_ARGUMENTS = ActiveStorage.unsupported_image_processing_arguments + class << self # Returns a Variation instance based on the given variator. If the variator is a Variation, it is # returned unmodified. If it is a String, it is passed to ActiveStorage::Variation.decode. Otherwise, @@ -56,12 +351,15 @@ def initialize(transformations) def transform(image) ActiveSupport::Notifications.instrument("transform.active_storage") do transformations.each do |name, argument_or_subtransformations| + validate_transformation(name, argument_or_subtransformations) image.mogrify do |command| if name.to_s == "combine_options" argument_or_subtransformations.each do |subtransformation_name, subtransformation_argument| + validate_transformation(subtransformation_name, subtransformation_argument) pass_transform_argument(command, subtransformation_name, subtransformation_argument) end else + validate_transformation(name, argument_or_subtransformations) pass_transform_argument(command, name, argument_or_subtransformations) end end @@ -86,4 +384,58 @@ def pass_transform_argument(command, method, argument) def eligible_argument?(argument) argument.present? && argument != true end + + def validate_transformation(name, argument) + method_name = name.to_s.gsub("-","_") + + unless SUPPORTED_IMAGE_PROCESSING_METHODS.any? { |method| method_name == method } + raise UnsupportedImageProcessingMethod, <<~ERROR.squish + One or more of the provided transformation methods is not supported. + ERROR + end + + if argument.present? + if argument.is_a?(String) || argument.is_a?(Symbol) + validate_arg_string(argument) + elsif argument.is_a?(Array) + validate_arg_array(argument) + elsif argument.is_a?(Hash) + validate_arg_hash(argument) + end + end + end + + def validate_arg_string(argument) + if UNSUPPORTED_IMAGE_PROCESSING_ARGUMENTS.any? { |bad_arg| argument.to_s.downcase.include?(bad_arg) }; raise UnsupportedImageProcessingArgument end + end + + def validate_arg_array(argument) + argument.each do |arg| + if arg.is_a?(Integer) || arg.is_a?(Float) + next + elsif arg.is_a?(String) || arg.is_a?(Symbol) + validate_arg_string(arg) + elsif arg.is_a?(Array) + validate_arg_array(arg) + elsif arg.is_a?(Hash) + validate_arg_hash(arg) + end + end + end + + def validate_arg_hash(argument) + argument.each do |key, value| + validate_arg_string(key) + + if value.is_a?(Integer) || value.is_a?(Float) + next + elsif value.is_a?(String) || value.is_a?(Symbol) + validate_arg_string(value) + elsif value.is_a?(Array) + validate_arg_array(value) + elsif value.is_a?(Hash) + validate_arg_hash(value) + end + end + end end diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 81c8c771dcce1..2b26d0c8258a6 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -50,4 +50,6 @@ module ActiveStorage mattr_accessor :content_types_to_serve_as_binary, default: [] mattr_accessor :content_types_allowed_inline, default: [] mattr_accessor :binary_content_type, default: "application/octet-stream" + mattr_accessor :supported_image_processing_methods, default: [] + mattr_accessor :unsupported_image_processing_arguments end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index a0a2982c7fa43..1702e5bc490be 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -51,6 +51,20 @@ class Engine < Rails::Engine # :nodoc: application/pdf ) + default_unsupported_image_processing_arguments = %w( + -debug + -display + -distribute-cache + -help + -path + -print + -set + -verbose + -version + -write + -write-mask + ) + config.eager_load_namespaces << ActiveStorage initializer "active_storage.configs" do @@ -61,6 +75,8 @@ class Engine < Rails::Engine # :nodoc: ActiveStorage.analyzers = app.config.active_storage.analyzers || [] ActiveStorage.paths = app.config.active_storage.paths || {} + ActiveStorage.supported_image_processing_methods = app.config.active_storage.supported_image_processing_methods || [] + ActiveStorage.unsupported_image_processing_arguments = app.config.active_storage.unsupported_image_processing_arguments || default_unsupported_image_processing_arguments ActiveStorage.variable_content_types = app.config.active_storage.variable_content_types || [] ActiveStorage.content_types_to_serve_as_binary = app.config.active_storage.content_types_to_serve_as_binary || [] ActiveStorage.content_types_allowed_inline = app.config.active_storage.content_types_allowed_inline || [] diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb index 1399d85c66edd..c44a037a06294 100644 --- a/activestorage/test/models/variant_test.rb +++ b/activestorage/test/models/variant_test.rb @@ -4,6 +4,80 @@ require "database/setup" class ActiveStorage::VariantTest < ActiveSupport::TestCase + test "variations with unsupported methods raise UnsupportedImageProcessingMethod" do + blob = create_file_blob(filename: "racecar.jpg") + assert_raise(ActiveStorage::Variation::UnsupportedImageProcessingMethod) do + blob.variant(system: "touch /tmp/dangerous").processed + end + end + + test "variations with unsupported methods in combine_options raise UnsupportedImageProcessingMethod" do + blob = create_file_blob(filename: "racecar.jpg") + + assert_raise(ActiveStorage::Variation::UnsupportedImageProcessingMethod) do + blob.variant(combine_options: { + gravity: "center", + write: "/tmp/danger", + crop: "100x10000", + }).processed + end + end + + test "variations with dangerous argument in combine_options raise UnsupportedImageProcessingArgument" do + blob = create_file_blob(filename: "racecar.jpg") + + assert_raise(ActiveStorage::Variation::UnsupportedImageProcessingArgument) do + blob.variant(combine_options: { + gravity: "center", + resize: "-write /tmp/danger", + crop: "100x10000", + }).processed + end + end + + test "variations with dangerous argument string raise UnsupportedImageProcessingArgument" do + blob = create_file_blob(filename: "racecar.jpg") + assert_raise(ActiveStorage::Variation::UnsupportedImageProcessingArgument) do + blob.variant(resize: "-PaTh /tmp/file.erb").processed + end + end + + test "variations with dangerous argument array raise UnsupportedImageProcessingArgument" do + blob = create_file_blob(filename: "racecar.jpg") + assert_raise(ActiveStorage::Variation::UnsupportedImageProcessingArgument) do + blob.variant(resize: [123, "-write", "/tmp/file.erb"]).processed + end + end + + test "variations with dangerous argument in a nested array raise UnsupportedImageProcessingArgument" do + blob = create_file_blob(filename: "racecar.jpg") + assert_raise(ActiveStorage::Variation::UnsupportedImageProcessingArgument) do + blob.variant(resize: [123, ["-write", "/tmp/file.erb"], "/tmp/file.erb"]).processed + end + + assert_raise(ActiveStorage::Variation::UnsupportedImageProcessingArgument) do + blob.variant(resize: [123, {"-write /tmp/file.erb": "something"}, "/tmp/file.erb"]).processed + end + end + + test "variations with dangerous argument hash raise UnsupportedImageProcessingArgument" do + blob = create_file_blob(filename: "racecar.jpg") + assert_raise(ActiveStorage::Variation::UnsupportedImageProcessingArgument) do + blob.variant(saver: {"-write": "/tmp/file.erb"}).processed + end + end + + test "variations with dangerous argument in a nested hash raise UnsupportedImageProcessingArgument" do + blob = create_file_blob(filename: "racecar.jpg") + assert_raise(ActiveStorage::Variation::UnsupportedImageProcessingArgument) do + blob.variant(saver: {"something": {"-write": "/tmp/file.erb"}}).processed + end + + assert_raise(ActiveStorage::Variation::UnsupportedImageProcessingArgument) do + blob.variant(saver: {"something": ["-write", "/tmp/file.erb"]}).processed + end + end + test "resized variation of JPEG blob" do blob = create_file_blob(filename: "racecar.jpg") variant = blob.variant(resize: "100x100").processed diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 9f7dcb38dc9a4..29963735a88fd 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1964,6 +1964,35 @@ def index assert_equal Digest::SHA1, ActiveSupport::Digest.hash_digest_class end + test "ActiveStorage.supported_image_processing_methods can be configured via config.active_storage.supported_image_processing_methods" do + remove_from_config '.*config\.load_defaults.*\n' + + app_file "config/initializers/add_image_processing_methods.rb", <<-RUBY + Rails.application.config.active_storage.supported_image_processing_methods = ["write", "set"] + RUBY + + app "development" + + assert ActiveStorage.supported_image_processing_methods.include?("write") + assert ActiveStorage.supported_image_processing_methods.include?("set") + end + + test "ActiveStorage.unsupported_image_processing_arguments can be configured via config.active_storage.unsupported_image_processing_arguments" do + remove_from_config '.*config\.load_defaults.*\n' + + app_file "config/initializers/add_image_processing_arguments.rb", <<-RUBY + Rails.application.config.active_storage.unsupported_image_processing_arguments = %w( + -write + -danger + ) + RUBY + + app "development" + + assert ActiveStorage.unsupported_image_processing_arguments.include?("-danger") + refute ActiveStorage.unsupported_image_processing_arguments.include?("-set") + end + private def force_lazy_load_hooks yield # Tasty clarifying sugar, homie! We only need to reference a constant to load it.