CVE-2023-32317: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Autolab is a course management service that enables auto-graded programming assignments. A Tar slip vulnerability was found in the MOSS cheat checker functionality of Autolab. To exploit this vulnerability an authenticated attacker with instructor permissions needs to upload a specially crafted Tar file. Both "Base File Tar" and "Additional file archive" can be fed with Tar files that contain paths outside their target directories (e.g.,  `../../../../tmp/tarslipped2.sh`). When the MOSS cheat checker is started the files inside of the archives are expanded to the attacker-chosen locations. This issue may lead to arbitrary file write within the scope of the running process.  This issue has been addressed in version 2.11.0. Users are advised to upgrade.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 7.2
Published: 2023-05-26T23:15Z
Repository: https://github.com/autolab/Autolab
Language: Ruby
Files changed: app/controllers/courses_controller.rb, lib/archive.rb
Commit date: 2023-05-17 02:32:33 -0400
Commit message: Merge pull request from GHSA-h8g5-vhm4-wx6g


Diff: diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb
index 1044828..d4756ea 100755
--- a/app/controllers/courses_controller.rb
+++ b/app/controllers/courses_controller.rb
@@ -1,8 +1,9 @@
 require "archive"
 require "csv"
 require "fileutils"
+require "pathname"
 require "statistics"
 
 class CoursesController < ApplicationController
   skip_before_action :set_course, only: %i[courses_redirect index new create]
   # you need to be able to pick a course to be authorized for it
@@ -1105,19 +1106,22 @@ private
       # write each file, renaming nested files
       archive_extract.each do |entry|
         pathname = Archive.get_entry_name(entry)
         next if Archive.looks_like_directory?(pathname)
 
-        destination = if archive
-                        File.join(extFilesDir,
-                                  pathname)
+        output_dir = if archive
+                       extFilesDir
                       else
-                        File.join(baseFilesDir, pathname)
+                        baseFilesDir
                       end
-        pathname.gsub!(%r{/}, "-")
+        output_file = File.join(output_dir, pathname)
+
+        # skip if the file lies outside the archive
+        next unless Archive.in_dir?(Pathname(output_file), Pathname(output_dir))
+
         # make sure all subdirectories are there
-        File.open(destination, "wb") do |out|
+        File.open(output_file, "wb") do |out|
           out.write Archive.read_entry_file(entry)
           begin
             out.fsync
           rescue StandardError
             nil
diff --git a/lib/archive.rb b/lib/archive.rb
index db07250..e859527 100644
--- a/lib/archive.rb
+++ b/lib/archive.rb
@@ -230,6 +230,17 @@ module Archive
   end
 
   def self.looks_like_directory?(pathname)
     pathname.ends_with?("/")
   end
+
+  # Check if a path lies (strictly) within a directory
+  # Takes two Pathname objects
+  # https://gist.github.com/henrik/48e8bb74de9d887770dfb3cc6efaa9b2
+  def self.in_dir?(path, dir)
+    path_parts = path.expand_path.each_filename.to_a
+    dir_parts = dir.expand_path.each_filename.to_a
+    return false if path_parts == dir_parts
+
+    dir_parts.zip(path_parts).all? { |x, y| x == y }
+  end
 end

Vulnerable code:         destination = if archive
                        File.join(extFilesDir,
                                  pathname)
                        File.join(baseFilesDir, pathname)
        pathname.gsub!(%r{/}, "-")
        File.open(destination, "wb") do |out|
Fixed code: require "pathname"
        output_dir = if archive
                       extFilesDir
                        baseFilesDir
        output_file = File.join(output_dir, pathname)

        # skip if the file lies outside the archive
        next unless Archive.in_dir?(Pathname(output_file), Pathname(output_dir))

        File.open(output_file, "wb") do |out|

  # Check if a path lies (strictly) within a directory
  # Takes two Pathname objects
  # https://gist.github.com/henrik/48e8bb74de9d887770dfb3cc6efaa9b2
  def self.in_dir?(path, dir)
    path_parts = path.expand_path.each_filename.to_a
    dir_parts = dir.expand_path.each_filename.to_a
    return false if path_parts == dir_parts

    dir_parts.zip(path_parts).all? { |x, y| x == y }
  end