From 99617771f43840dc12e1e2316dd112afb72b7dec Mon Sep 17 00:00:00 2001
From: Justin Coyne <jcoyne@justincoyne.com>
Date: Mon, 4 Dec 2023 11:05:39 -0600
Subject: [PATCH] Sanitize inputs

Don't cast user input to float or integer without sanitizing
---
 .../legacy_image_service_controller.rb        | 28 +++++++++++++------
 config/environments/test.rb                   |  2 +-
 .../legacy_image_service_controller_spec.rb   |  5 ++--
 spec/requests/legacy_image_service_spec.rb    | 25 +++++++++++++++++
 4 files changed, 49 insertions(+), 11 deletions(-)
 create mode 100644 spec/requests/legacy_image_service_spec.rb

diff --git a/app/controllers/legacy_image_service_controller.rb b/app/controllers/legacy_image_service_controller.rb
index 0752a13a..44026e9e 100644
--- a/app/controllers/legacy_image_service_controller.rb
+++ b/app/controllers/legacy_image_service_controller.rb
@@ -44,8 +44,8 @@ def iiif_params
 
   def iiif_size
     case
-    when zoom
-      "pct:#{zoom}"
+    when allowed_params[:zoom]
+      "pct:#{allowed_params[:zoom]}"
     when allowed_params[:w]
       "#{allowed_params[:w]},#{allowed_params[:h]}"
     when size
@@ -71,11 +71,10 @@ def iiif_size
   end
 
   def iiif_region
+    zoomed_region = Region.new(params[:region], params[:zoom]) if params[:region] && params[:zoom]
     case
-    when region && zoom
-      x, y, w, h = region.split(',')
-      zoom_percent = zoom.to_f / 100.0
-      [x.to_i / zoom_percent, y.to_i / zoom_percent, w.to_i / zoom_percent, h.to_i / zoom_percent].map(&:to_i).join(',')
+    when zoomed_region
+      zoomed_region.to_iiif_region
     when region
       region
     when size == 'square'
@@ -105,7 +104,20 @@ def size
     allowed_params[:size]
   end
 
-  def zoom
-    allowed_params[:zoom]
+  # A subset of an image defined by a region and zoom level
+  class Region
+    def initialize(raw_region, raw_zoom)
+      raise ActionController::RoutingError, 'zoom is invalid' unless /\A\d*\.?\d+\z/.match?(raw_zoom)
+      raise ActionController::RoutingError, 'region is invalid' unless /\A(\d+,){0,3}\d+\z/.match?(raw_region)
+
+      @zoom_percent = raw_zoom.to_f / 100.0
+      @x, @y, @w, @h = raw_region.split(',')
+    end
+
+    attr_reader :zoom_percent, :x, :y, :w, :h
+
+    def to_iiif_region
+      [x.to_i / zoom_percent, y.to_i / zoom_percent, w.to_i / zoom_percent, h.to_i / zoom_percent].map(&:to_i).join(',')
+    end
   end
 end
diff --git a/config/environments/test.rb b/config/environments/test.rb
index eb2f1716..3fd3e699 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -28,7 +28,7 @@
   config.cache_store = :null_store
 
   # Raise exceptions instead of rendering exception templates.
-  config.action_dispatch.show_exceptions = false
+  config.action_dispatch.show_exceptions = true
 
   # Disable request forgery protection in test environment.
   config.action_controller.allow_forgery_protection = false
diff --git a/spec/controllers/legacy_image_service_controller_spec.rb b/spec/controllers/legacy_image_service_controller_spec.rb
index 30595577..3ee7503f 100644
--- a/spec/controllers/legacy_image_service_controller_spec.rb
+++ b/spec/controllers/legacy_image_service_controller_spec.rb
@@ -53,7 +53,8 @@
       end
     end
   end
-  context 'delivering tiles' do
+
+  describe 'delivering tiles' do
     it 'works at 100% zoom' do
       page = get :show, params: { id: 'nr349ct7889',
                                   file_name: 'nr349ct7889_00_0001',
@@ -63,7 +64,7 @@
       expect(page).to redirect_to '/image/iiif/nr349ct7889/nr349ct7889_00_0001/0,0,256,256/pct:100/0/default.jpg'
     end
 
-    it 'works at 50% zome' do
+    it 'works at 50% zoom' do
       page = get :show, params: { id: 'nr349ct7889',
                                   file_name: 'nr349ct7889_00_0001',
                                   format: 'jpg',
diff --git a/spec/requests/legacy_image_service_spec.rb b/spec/requests/legacy_image_service_spec.rb
new file mode 100644
index 00000000..d28daeb9
--- /dev/null
+++ b/spec/requests/legacy_image_service_spec.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Legacy image service' do
+  context 'with an invalid zoom value' do
+    before do
+      get '/image/nr349ct7889/nr349ct7889_00_0001.jpg?zoom=test&region=256,256,256,256'
+    end
+
+    it 'is not found' do
+      expect(response).to have_http_status(:not_found)
+    end
+  end
+
+  context 'with an invalid region value' do
+    before do
+      get '/image/nr349ct7889/nr349ct7889_00_0001.jpg?zoom=50&region=test'
+    end
+
+    it 'is not found' do
+      expect(response).to have_http_status(:not_found)
+    end
+  end
+end