Merge pull request #129 from loftwah/dl/security-fix-1

patch security vulnerabilities
loftwah · Sep 8, 2024 · 13890ce · 13890ce
2 parents 3216382 + 0b7229a
commit 13890ce
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 16 deletions.
diff --git a/app/models/user.rb b/app/models/user.rb
@@ -42,26 +42,28 @@ def generate_open_graph_image
   def download_and_store_avatar
     return if avatar.blank?
 
-    require 'open-uri'
-    require 'fileutils'
-
     begin
       avatar_dir = Rails.root.join('public', 'avatars')
       FileUtils.mkdir_p(avatar_dir) unless File.directory?(avatar_dir)
 
-      file = URI.open(avatar)
-
+      uri = URI.parse(avatar)
       filename = "#{username}_avatar#{File.extname(avatar)}"
       filepath = File.join(avatar_dir, filename)
 
-      File.open(filepath, 'wb') do |local_file|
-        local_file.write(file.read)
+      response = Net::HTTP.get_response(uri)
+      if response.is_a?(Net::HTTPSuccess)
+        File.open(filepath, 'wb') do |local_file|
+          local_file.write(response.body)
+        end
+        Rails.logger.info "Avatar downloaded for user #{username}"
+      else
+        Rails.logger.error "Failed to download avatar for user #{username}. HTTP Error: #{response.code} #{response.message}. Using default avatar."
+        self.avatar = 'greg.jpg'  # Set to default avatar
+        save(validate: false)  # Save without triggering validations
       end
-
-      Rails.logger.info "Avatar downloaded for user #{username}"
     rescue StandardError => e
       Rails.logger.error "Failed to download avatar for user #{username}: #{e.message}"
-      self.avatar = 'greg.jpg'  # Set to default avatar instead of nil
+      self.avatar = 'greg.jpg'  # Set to default avatar
       save(validate: false)  # Save without triggering validations
     end
   end

diff --git a/app/services/open_graph_image_generator.rb b/app/services/open_graph_image_generator.rb
@@ -54,17 +54,22 @@ def generate
   private
 
   def download_image(url)
+    uri = URI.parse(url)
     tempfile = Tempfile.new(['avatar', '.jpg'])
     tempfile.binmode
     begin
-      URI.open(url) do |image|
-        tempfile.write(image.read)
+      response = Net::HTTP.get_response(uri)
+      if response.is_a?(Net::HTTPSuccess)
+        tempfile.write(response.body)
+        tempfile.rewind
+        MiniMagick::Image.open(tempfile.path)
+      else
+        Rails.logger.error("Failed to download image from URL: #{url}. HTTP Error: #{response.code} #{response.message}. Using default avatar.")
+        MiniMagick::Image.open(default_avatar_path)
       end
-      tempfile.rewind
-      MiniMagick::Image.open(tempfile.path)
-    rescue OpenURI::HTTPError, Errno::ENOENT, SocketError => e
+    rescue SocketError, Errno::ENOENT => e
       Rails.logger.error("Failed to download image from URL: #{url}. Error: #{e.message}. Using default avatar.")
-      MiniMagick::Image.open(default_avatar_path) # Use default avatar
+      MiniMagick::Image.open(default_avatar_path)
     ensure
       tempfile.close
       tempfile.unlink  # Unlink after we've processed the image