diff --git a/Library/Homebrew/dev-cmd/generate-vulns-advisories.rb b/Library/Homebrew/dev-cmd/generate-vulns-advisories.rb index 07e192ee7a..de06037d1e 100644 --- a/Library/Homebrew/dev-cmd/generate-vulns-advisories.rb +++ b/Library/Homebrew/dev-cmd/generate-vulns-advisories.rb @@ -3,6 +3,7 @@ require "abstract_command" require "formula" +require "formula_versions" require "vulns/osv_export" module Homebrew @@ -32,10 +33,10 @@ module Homebrew dir = args.named.first Formulary.enable_factory_cache! - annotated = Homebrew.with_no_api_env do + Homebrew.with_no_api_env do latest_macos = MacOSVersion.new((HOMEBREW_MACOS_NEWEST_UNSUPPORTED.to_i - 1).to_s).to_sym Homebrew::SimulateSystem.with(os: latest_macos, arch: :arm) do - tap.formula_names.filter_map do |name| + annotated = tap.formula_names.filter_map do |name| formula = Formulary.factory(name) patches = all_variation_patches(formula) [formula, patches] if Homebrew::Vulns::Scanner.resolved_ids(patches).any? @@ -43,22 +44,25 @@ module Homebrew onoe "Error loading formula '#{name}'." raise end - end - end - ohai "#{annotated.size} formulae with security `resolves` annotations" + ohai "#{annotated.size} formulae with security `resolves` annotations" - if args.dry_run? - annotated.each do |formula, patches| - Homebrew::Vulns::Scanner.resolved_ids(patches).each do |vuln_id| - puts "#{Homebrew::Vulns::OsvExport::ID_PREFIX}-#{formula.name}-#{vuln_id}" + if args.dry_run? + annotated.each do |formula, patches| + Homebrew::Vulns::Scanner.resolved_ids(patches).each do |vuln_id| + puts "#{Homebrew::Vulns::OsvExport::ID_PREFIX}-#{formula.name}-#{vuln_id}" + end + end + next end - end - return - end - written = Homebrew::Vulns::OsvExport.run(annotated, T.must(dir)) - written.each { |p| puts " wrote #{p}" } if args.verbose? - ohai "#{written.size} records written to #{dir}" + written = Homebrew::Vulns::OsvExport.run( + annotated, T.must(dir), + first_fixed: ->(formula, vuln_id) { first_fixed_version(formula, vuln_id) } + ) + written.each { |p| puts " wrote #{p}" } if args.verbose? + ohai "#{written.size} records written to #{dir}" + end + end end # `Formula#serialized_patches` reflects the currently simulated OS and @@ -74,6 +78,52 @@ module Homebrew variation_patches = hash.fetch("variations").values.filter_map { |v| v["patches"] } (base + variation_patches.flatten(1)).uniq end + + # Walk homebrew-core git history (newest first) via {FormulaVersions} and + # return the `pkg_version` at the oldest revision where `vuln_id` still + # appears in the formula's resolved patch ids: the version at which the + # fix first shipped. Revisions that fail to load (older DSL) end the walk + # early. Only invoked for records with no existing file, so the cost is + # bounded to newly annotated (formula, CVE) pairs. + # + # Because `resolved_ids` includes CVEs inferred from patch URLs and + # `apply` file paths, this finds the true fix version when the CVE is + # named there. When a `resolves` line was added to a patch that had + # already shipped without a CVE reference, it finds when `resolves` was + # added (too recent); those cases are hand-corrected in the advisory + # repository, which {Homebrew::Vulns::OsvExport.run} then preserves. + # + # Historical revisions are loaded under the enclosing {SimulateSystem} + # (latest macOS/ARM) only; a `resolves` that lives inside e.g. `on_linux` + # is invisible here and falls through to the current `pkg_version`. + # {FormulaVersions} caches by revision alone, so per-variation historical + # loading would need separate instances; deferred until a variation-only + # security annotation actually exists in core. + sig { params(formula: Formula, vuln_id: String).returns(T.nilable(String)) } + def first_fixed_version(formula, vuln_id) + # `FormulaVersions#rev_list` shells out to path-filtered `git rev-list` + # over the whole homebrew-core history and dominates runtime; cache it + # (and the instance, for its per-revision formula memoisation) per + # formula so subsequent CVEs for the same formula reuse both. + @formula_versions ||= T.let({}, T.nilable(T::Hash[String, FormulaVersions])) + @formula_rev_lists ||= T.let({}, T.nilable(T::Hash[String, T::Array[[String, String]]])) + fv = @formula_versions[formula.name] ||= FormulaVersions.new(formula) + revs = @formula_rev_lists[formula.name] ||= + [].tap { |a| fv.rev_list("HEAD") { |rev, entry| a << [rev, entry] } } + + last_fixed = T.let(nil, T.nilable(String)) + revs.each do |rev, entry| + resolved_here = fv.formula_at_revision(rev, entry) do |old| + Homebrew::Vulns::Scanner.resolved_ids(old.serialized_patches).include?(vuln_id) + end + # `nil` means the revision failed to load; stop rather than guess. + return last_fixed if resolved_here.nil? + return last_fixed unless resolved_here + + last_fixed = fv.formula_at_revision(rev, entry) { |old| old.pkg_version.to_s } + end + last_fixed + end end end end diff --git a/Library/Homebrew/test/dev-cmd/generate-vulns-advisories_spec.rb b/Library/Homebrew/test/dev-cmd/generate-vulns-advisories_spec.rb index 562bc292fd..9c3354ba50 100644 --- a/Library/Homebrew/test/dev-cmd/generate-vulns-advisories_spec.rb +++ b/Library/Homebrew/test/dev-cmd/generate-vulns-advisories_spec.rb @@ -30,6 +30,7 @@ RSpec.describe Homebrew::DevCmd::GenerateVulnsAdvisories do allow(Formulary).to receive(:factory).with("nvi").and_return(nvi) allow(Formulary).to receive(:factory).with("plain").and_return(plain) allow(Homebrew::Vulns::OSV).to receive(:vulnerability).and_return({}) + allow(FormulaVersions).to receive(:new).and_return(instance_double(FormulaVersions, rev_list: nil)) Dir.mktmpdir do |dir| out = "#{dir}/advisories" @@ -97,4 +98,68 @@ RSpec.describe Homebrew::DevCmd::GenerateVulnsAdvisories do expect(cmd.all_variation_patches(f)).to eq [{ "url" => "a" }] end end + + describe "#first_fixed_version" do + subject(:cmd) { described_class.new(["out"]) } + + let(:current) { formula("x") { url "https://example.com/x-1.2.tar.gz" } } + + def with_history(revisions) + fv = instance_double(FormulaVersions) + allow(FormulaVersions).to receive(:new).with(current).and_return(fv) + allow(fv).to receive(:rev_list).with("HEAD") do |&blk| + revisions.each_key { |rev| blk.call(rev, "Formula/x/x.rb") } + end + allow(fv).to receive(:formula_at_revision) do |rev, _entry, &blk| + old = revisions.fetch(rev) + old.nil? ? nil : blk.call(old) + end + end + + def old_formula(pkg_version:, resolves_ids: []) + patches = resolves_ids.map { |id| { "resolves" => [{ "type" => "security", "id" => id }] } } + instance_double(Formula, pkg_version: PkgVersion.parse(pkg_version), serialized_patches: patches) + end + + it "returns the pkg_version at the oldest revision where the CVE is resolved" do + with_history( + "r3" => old_formula(pkg_version: "1.2_1", resolves_ids: ["CVE-2024-1"]), + "r2" => old_formula(pkg_version: "1.2", resolves_ids: ["CVE-2024-1"]), + "r1" => old_formula(pkg_version: "1.1", resolves_ids: []), + # Trap: if the walk continued past r1 it would wrongly return 1.0. + "r0" => old_formula(pkg_version: "1.0", resolves_ids: ["CVE-2024-1"]), + ) + + expect(cmd.first_fixed_version(current, "CVE-2024-1")).to eq "1.2" + end + + it "returns the oldest resolved version when the CVE is resolved in every revision" do + with_history( + "r2" => old_formula(pkg_version: "1.1", resolves_ids: ["CVE-2024-1"]), + "r1" => old_formula(pkg_version: "1.0", resolves_ids: ["CVE-2024-1"]), + ) + + expect(cmd.first_fixed_version(current, "CVE-2024-1")).to eq "1.0" + end + + it "stops at an unloadable revision and returns the last known resolved version" do + with_history( + "r3" => old_formula(pkg_version: "1.2", resolves_ids: ["CVE-2024-1"]), + "r2" => nil, + "r1" => old_formula(pkg_version: "1.0", resolves_ids: ["CVE-2024-1"]), + ) + + expect(cmd.first_fixed_version(current, "CVE-2024-1")).to eq "1.2" + end + + it "returns nil when the CVE is not resolved at the newest revision" do + with_history( + "r2" => old_formula(pkg_version: "1.2", resolves_ids: []), + # Trap: if the walk continued past r2 it would wrongly return 1.0. + "r1" => old_formula(pkg_version: "1.0", resolves_ids: ["CVE-2024-1"]), + ) + + expect(cmd.first_fixed_version(current, "CVE-2024-1")).to be_nil + end + end end diff --git a/Library/Homebrew/test/vulns/osv_export_spec.rb b/Library/Homebrew/test/vulns/osv_export_spec.rb index 2a2f998fa7..2866757544 100644 --- a/Library/Homebrew/test/vulns/osv_export_spec.rb +++ b/Library/Homebrew/test/vulns/osv_export_spec.rb @@ -154,6 +154,13 @@ RSpec.describe Homebrew::Vulns::OsvExport do expect(affected[:package][:name]).to eq "libsigc++" end + it "uses an explicit fixed boundary over the current pkg_version" do + events = described_class.record_for(nvi, "CVE-2015-2305", fixed: "1.81.6_3", now:) + .dig(:affected, 0, :ranges, 0, :events) + + expect(events[1]).to eq({ fixed: "1.81.6_3" }) + end + it "omits the revision suffix when revision is zero" do f = formula("x") do url "https://example.com/x-1.0.tar.gz" @@ -190,6 +197,37 @@ RSpec.describe Homebrew::Vulns::OsvExport do end end + it "calls first_fixed only for records with no existing file" do + allow(Homebrew::Vulns::OSV).to receive(:vulnerability).and_return({}) + calls = [] + first_fixed = lambda { |f, id| + calls << [f.name, id] + "1.2.4_2" + } + + Dir.mktmpdir do |dir| + File.write("#{dir}/BREW-libquicktime-CVE-2016-2399.json", + JSON.generate(described_class.record_for(libquicktime, "CVE-2016-2399", now:))) + + described_class.run([[libquicktime, libquicktime.serialized_patches]], dir, first_fixed:, now:) + + expect(calls).to eq [["libquicktime", "CVE-2017-9122"]] + record = JSON.parse(File.read("#{dir}/BREW-libquicktime-CVE-2017-9122.json")) + expect(record["affected"][0]["ranges"][0]["events"][1]).to eq({ "fixed" => "1.2.4_2" }) + end + end + + it "falls back to pkg_version when first_fixed returns nil" do + allow(Homebrew::Vulns::OSV).to receive(:vulnerability).and_return({}) + + Dir.mktmpdir do |dir| + described_class.run([[nvi, nvi.serialized_patches]], dir, first_fixed: ->(_f, _id) {}, now:) + + record = JSON.parse(File.read("#{dir}/BREW-nvi-CVE-2015-2305.json")) + expect(record["affected"][0]["ranges"][0]["events"][1]).to eq({ "fixed" => "1.81.6_6" }) + end + end + it "fetches each upstream vuln id once, even when shared across formulae" do shared = [{ "url" => "https://example.com/fix.patch", "resolves" => [{ "type" => "security", "id" => "CVE-2024-9999" }] }] diff --git a/Library/Homebrew/vulns/osv_export.rb b/Library/Homebrew/vulns/osv_export.rb index 53d009b066..303165e36a 100644 --- a/Library/Homebrew/vulns/osv_export.rb +++ b/Library/Homebrew/vulns/osv_export.rb @@ -38,12 +38,19 @@ module Homebrew # supply the union across OS/architecture variations (a `patch` inside an # `on_linux`/`on_intel` block only appears in `Formula#serialized_patches` # under the matching {SimulateSystem}). + # + # `first_fixed`, when given, is called `(formula, vuln_id) -> String?` for + # records with no existing file to derive an accurate `fixed` boundary + # (e.g. via {FormulaVersions} git history); existing records preserve + # their on-disk `ranges` regardless. sig { - params(annotated: T::Array[[Formula, T::Array[T::Hash[String, T.untyped]]]], - dir: T.any(String, Pathname), now: Time) + params(annotated: T::Array[[Formula, T::Array[T::Hash[String, T.untyped]]]], + dir: T.any(String, Pathname), + first_fixed: T.nilable(T.proc.params(formula: Formula, vuln_id: String).returns(T.nilable(String))), + now: Time) .returns(T::Array[String]) } - def self.run(annotated, dir, now: Time.now.utc) + def self.run(annotated, dir, first_fixed: nil, now: Time.now.utc) FileUtils.mkdir_p(dir) written = [] upstream_cache = T.let({}, T::Hash[String, T.any(T::Hash[String, T.untyped], Symbol)]) @@ -52,11 +59,13 @@ module Homebrew Scanner.resolved_ids(patches).each do |vuln_id| upstream = upstream_cache.fetch(vuln_id) { upstream_cache[vuln_id] = fetch_upstream(vuln_id) } path = File.join(dir, "#{ID_PREFIX}-#{formula.name}-#{vuln_id}.json") + existing = File.file?(path) # A transient OSV outage would otherwise strip summary/severity/etc. # from an existing enriched record; leave it untouched instead. - next if upstream == :failed && File.file?(path) + next if upstream == :failed && existing - record = record_for(formula, vuln_id, patches:, + fixed = (first_fixed&.call(formula, vuln_id) unless existing) || formula.pkg_version.to_s + record = record_for(formula, vuln_id, patches:, fixed:, upstream: upstream.is_a?(Hash) ? upstream : nil, now:) merged = merge_existing(path, record) next if merged.nil? @@ -105,10 +114,11 @@ module Homebrew sig { params(formula: Formula, vuln_id: String, patches: T::Array[T::Hash[String, T.untyped]], - upstream: T.nilable(T::Hash[String, T.untyped]), now: Time) + fixed: String, upstream: T.nilable(T::Hash[String, T.untyped]), now: Time) .returns(T::Hash[Symbol, T.untyped]) } - def self.record_for(formula, vuln_id, patches: formula.serialized_patches, upstream: nil, now: Time.now.utc) + def self.record_for(formula, vuln_id, patches: formula.serialized_patches, + fixed: formula.pkg_version.to_s, upstream: nil, now: Time.now.utc) timestamp = now.strftime("%Y-%m-%dT%H:%M:%SZ") record = T.let({ schema_version: SCHEMA_VERSION, @@ -116,7 +126,7 @@ module Homebrew published: timestamp, modified: timestamp, upstream: [vuln_id], - affected: [affected_entry(formula, vuln_id, patches)], + affected: [affected_entry(formula, vuln_id, patches, fixed)], database_specific: { source: "generated" }, }, T::Hash[Symbol, T.untyped]) @@ -132,10 +142,10 @@ module Homebrew end sig { - params(formula: Formula, vuln_id: String, patches: T::Array[T::Hash[String, T.untyped]]) + params(formula: Formula, vuln_id: String, patches: T::Array[T::Hash[String, T.untyped]], fixed: String) .returns(T::Hash[Symbol, T.untyped]) } - def self.affected_entry(formula, vuln_id, patches) + def self.affected_entry(formula, vuln_id, patches, fixed) { package: { ecosystem: ECOSYSTEM, @@ -145,7 +155,7 @@ module Homebrew ranges: [ { type: "ECOSYSTEM", - events: [{ introduced: "0" }, { fixed: formula.pkg_version.to_s }], + events: [{ introduced: "0" }, { fixed: }], }, ], ecosystem_specific: {