diff --git a/modules/bim/lib/open_project/bim/bcf_xml/exporter.rb b/modules/bim/lib/open_project/bim/bcf_xml/exporter.rb index eaded040021..5abc89c1389 100644 --- a/modules/bim/lib/open_project/bim/bcf_xml/exporter.rb +++ b/modules/bim/lib/open_project/bim/bcf_xml/exporter.rb @@ -2,6 +2,10 @@ require "fileutils" module OpenProject::Bim::BcfXml class Exporter < ::WorkPackage::Exports::QueryExporter + # Strict UUID format used to protect against path traversal when building + # filesystem paths (issue folders, viewpoint files) from BCF GUIDs. + UUID_REGEX = /\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/i + def initialize(object, options = {}) object.add_filter("bcf_issue_associated", "=", ["t"]) super @@ -96,6 +100,12 @@ module OpenProject::Bim::BcfXml # Create and return the issue folder # /dir// def topic_folder_for(dir, issue) + # Sanity check for the issue GUID, to protect against path traversal + # when generating the issue folder name. + unless issue.uuid.to_s.match?(UUID_REGEX) + raise ArgumentError, "Refusing to export BCF issue with invalid uuid: #{issue.uuid.inspect}" + end + File.join(dir, issue.uuid).tap do |issue_dir| Dir.mkdir issue_dir end @@ -116,8 +126,7 @@ module OpenProject::Bim::BcfXml issue.viewpoints.find_each do |vp| # Sanity check for the viewpoints GUID, to protect against # path traversal when generating the viewpoint file name - uuid_regex = /\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/i - next unless vp.uuid.match?(uuid_regex) + next unless vp.uuid.to_s.match?(UUID_REGEX) vp_file = File.join(issue_dir, "#{vp.uuid}.bcfv") snapshot_file = File.join(issue_dir, "#{vp.uuid}#{vp.snapshot.extension}") diff --git a/modules/bim/spec/bcf/bcf_xml/exporter_spec.rb b/modules/bim/spec/bcf/bcf_xml/exporter_spec.rb index 40373b81eac..4032940bb58 100644 --- a/modules/bim/spec/bcf/bcf_xml/exporter_spec.rb +++ b/modules/bim/spec/bcf/bcf_xml/exporter_spec.rb @@ -58,4 +58,24 @@ RSpec.describe OpenProject::Bim::BcfXml::Exporter do expect(subject.work_packages.count).to be(1) end end + + describe "#topic_folder_for" do + let(:dir) { Dir.mktmpdir } + + after { FileUtils.remove_entry(dir) } + + it "creates a folder for a valid uuid" do + issue = instance_double(Bim::Bcf::Issue, uuid: SecureRandom.uuid) + folder = subject.send(:topic_folder_for, dir, issue) + expect(File.directory?(folder)).to be(true) + expect(folder).to eq(File.join(dir, issue.uuid)) + end + + it "raises rather than creating a folder for an invalid uuid" do + issue = instance_double(Bim::Bcf::Issue, uuid: "../../../../tmp/") + expect { subject.send(:topic_folder_for, dir, issue) } + .to raise_error(ArgumentError, /invalid uuid/) + expect(Dir.children(dir)).to be_empty + end + end end