From 5aebde3ffdb38a7087ac992af144c957dd802788 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Mon, 1 Jan 2024 17:46:48 +0000 Subject: [PATCH] Add consistent path validation --- Library/Homebrew/dev-cmd/bottle.rb | 12 ++++----- Library/Homebrew/download_strategy.rb | 6 ++--- Library/Homebrew/formula.rb | 6 +++-- Library/Homebrew/software_spec.rb | 6 +++++ Library/Homebrew/test/bottle_filename_spec.rb | 26 +++++++++---------- Library/Homebrew/utils.rb | 13 ++++++++++ 6 files changed, 45 insertions(+), 24 deletions(-) diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb index 6a51a21e1e..524674708d 100644 --- a/Library/Homebrew/dev-cmd/bottle.rb +++ b/Library/Homebrew/dev-cmd/bottle.rb @@ -365,9 +365,9 @@ module Homebrew end || 0 end - filename = Bottle::Filename.create(formula, bottle_tag.to_sym, rebuild) + filename = Bottle::Filename.create(formula, bottle_tag, rebuild) local_filename = filename.to_s - bottle_path = Pathname.pwd/filename + bottle_path = Pathname.pwd/local_filename tab = nil keg = nil @@ -690,8 +690,8 @@ module Homebrew bottle_hash["bottle"]["tags"].each do |tag, tag_hash| filename = Bottle::Filename.new( formula_name, - bottle_hash["formula"]["pkg_version"], - tag, + PkgVersion.parse(bottle_hash["formula"]["pkg_version"]), + Utils::Bottles::Tag.from_symbol(tag.to_sym), bottle_hash["bottle"]["rebuild"], ) @@ -700,8 +700,8 @@ module Homebrew all_filename = Bottle::Filename.new( formula_name, - bottle_hash["formula"]["pkg_version"], - "all", + PkgVersion.parse(bottle_hash["formula"]["pkg_version"]), + Utils::Bottles::Tag.from_symbol(:all), bottle_hash["bottle"]["rebuild"], ) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index dc486de7a8..4f9becfad9 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -197,7 +197,7 @@ class VCSDownloadStrategy < AbstractDownloadStrategy super @ref_type, @ref = extract_ref(meta) @revision = meta[:revision] - @cached_location = @cache/"#{name}--#{cache_tag}" + @cached_location = @cache/Utils.safe_filename("#{name}--#{cache_tag}") end # Download and cache the repository at {#cached_location}. @@ -296,7 +296,7 @@ class AbstractFileDownloadStrategy < AbstractDownloadStrategy return @symlink_location if defined?(@symlink_location) ext = Pathname(parse_basename(url)).extname - @symlink_location = @cache/"#{name}--#{version}#{ext}" + @symlink_location = @cache/Utils.safe_filename("#{name}--#{version}#{ext}") end # Path for storing the completed download. @@ -312,7 +312,7 @@ class AbstractFileDownloadStrategy < AbstractDownloadStrategy @cached_location = if downloads.count == 1 downloads.first else - HOMEBREW_CACHE/"downloads/#{url_sha256}--#{resolved_basename}" + HOMEBREW_CACHE/"downloads/#{url_sha256}--#{Utils.safe_filename(resolved_basename)}" end end diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 77be731c68..5782013a50 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -307,13 +307,15 @@ class Formula end def validate_attributes! - raise FormulaValidationError.new(full_name, :name, name) if name.blank? || name.match?(/\s/) + if name.blank? || name.match?(/\s/) || !Utils.safe_filename?(name) + raise FormulaValidationError.new(full_name, :name, name) + end url = active_spec.url raise FormulaValidationError.new(full_name, :url, url) if url.blank? || url.match?(/\s/) val = version.respond_to?(:to_str) ? version.to_str : version - return if val.present? && !val.match?(/\s/) + return if val.present? && !val.match?(/\s/) && Utils.safe_filename?(val) raise FormulaValidationError.new(full_name, :version, val) end diff --git a/Library/Homebrew/software_spec.rb b/Library/Homebrew/software_spec.rb index 1d47f14c85..a55600ee68 100644 --- a/Library/Homebrew/software_spec.rb +++ b/Library/Homebrew/software_spec.rb @@ -294,12 +294,18 @@ class Bottle class Filename attr_reader :name, :version, :tag, :rebuild + sig { params(formula: Formula, tag: Utils::Bottles::Tag, rebuild: Integer).returns(T.attached_class) } def self.create(formula, tag, rebuild) new(formula.name, formula.pkg_version, tag, rebuild) end + sig { params(name: String, version: PkgVersion, tag: Utils::Bottles::Tag, rebuild: Integer).void } def initialize(name, version, tag, rebuild) @name = File.basename name + + raise ArgumentError, "Invalid bottle name" unless Utils.safe_filename?(@name) + raise ArgumentError, "Invalid bottle version" unless Utils.safe_filename?(version.to_s) + @version = version @tag = tag.to_s @rebuild = rebuild diff --git a/Library/Homebrew/test/bottle_filename_spec.rb b/Library/Homebrew/test/bottle_filename_spec.rb index 9eef4089c9..c8e7a27591 100644 --- a/Library/Homebrew/test/bottle_filename_spec.rb +++ b/Library/Homebrew/test/bottle_filename_spec.rb @@ -7,47 +7,47 @@ describe Bottle::Filename do subject { described_class.new(name, version, tag, rebuild) } let(:name) { "user/repo/foo" } - let(:version) { "1.0" } - let(:tag) { :tag } + let(:version) { PkgVersion.new(Version.new("1.0"), 0) } + let(:tag) { Utils::Bottles::Tag.from_symbol(:x86_64_linux) } let(:rebuild) { 0 } describe "#extname" do - its(:extname) { is_expected.to eq ".tag.bottle.tar.gz" } + its(:extname) { is_expected.to eq ".x86_64_linux.bottle.tar.gz" } context "when rebuild is 0" do - its(:extname) { is_expected.to eq ".tag.bottle.tar.gz" } + its(:extname) { is_expected.to eq ".x86_64_linux.bottle.tar.gz" } end context "when rebuild is 1" do let(:rebuild) { 1 } - its(:extname) { is_expected.to eq ".tag.bottle.1.tar.gz" } + its(:extname) { is_expected.to eq ".x86_64_linux.bottle.1.tar.gz" } end end describe "#to_s and #to_str" do - its(:to_s) { is_expected.to eq "foo--1.0.tag.bottle.tar.gz" } - its(:to_str) { is_expected.to eq "foo--1.0.tag.bottle.tar.gz" } + its(:to_s) { is_expected.to eq "foo--1.0.x86_64_linux.bottle.tar.gz" } + its(:to_str) { is_expected.to eq "foo--1.0.x86_64_linux.bottle.tar.gz" } end describe "#url_encode" do - its(:url_encode) { is_expected.to eq "foo-1.0.tag.bottle.tar.gz" } + its(:url_encode) { is_expected.to eq "foo-1.0.x86_64_linux.bottle.tar.gz" } end describe "#github_packages" do - its(:github_packages) { is_expected.to eq "foo--1.0.tag.bottle.tar.gz" } + its(:github_packages) { is_expected.to eq "foo--1.0.x86_64_linux.bottle.tar.gz" } end describe "#json" do - its(:json) { is_expected.to eq "foo--1.0.tag.bottle.json" } + its(:json) { is_expected.to eq "foo--1.0.x86_64_linux.bottle.json" } context "when rebuild is 1" do - its(:json) { is_expected.to eq "foo--1.0.tag.bottle.json" } + its(:json) { is_expected.to eq "foo--1.0.x86_64_linux.bottle.json" } end end describe "::create" do - subject { described_class.create(f, :tag, 0) } + subject { described_class.create(f, tag, rebuild) } let(:f) do formula do @@ -56,6 +56,6 @@ describe Bottle::Filename do end end - its(:to_s) { is_expected.to eq "formula_name--1.0.tag.bottle.tar.gz" } + its(:to_s) { is_expected.to eq "formula_name--1.0.x86_64_linux.bottle.tar.gz" } end end diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb index 74b1f83c24..b8366488b2 100644 --- a/Library/Homebrew/utils.rb +++ b/Library/Homebrew/utils.rb @@ -168,4 +168,17 @@ module Utils word.downcase! word end + + SAFE_FILENAME_REGEX = /[[:cntrl:]#{Regexp.escape("#{File::SEPARATOR}#{File::ALT_SEPARATOR}")}]/o + private_constant :SAFE_FILENAME_REGEX + + sig { params(basename: String).returns(T::Boolean) } + def self.safe_filename?(basename) + !SAFE_FILENAME_REGEX.match?(basename) + end + + sig { params(basename: String).returns(String) } + def self.safe_filename(basename) + basename.gsub(SAFE_FILENAME_REGEX, "") + end end