diff --git a/app/models/journable/historic_active_record_relation.rb b/app/models/journable/historic_active_record_relation.rb index a28902cb9a1..91b66023153 100644 --- a/app/models/journable/historic_active_record_relation.rb +++ b/app/models/journable/historic_active_record_relation.rb @@ -229,7 +229,8 @@ class Journable::HistoricActiveRecordRelation < ActiveRecord::Relation raise NoMethodError, "Unknown timestamp type: #{timestamp.class}" end - "WHEN \"#{Journal.table_name}\".\"validity_period\" @> timestamp with time zone '#{comparison_time}' THEN '#{timestamp}'" + quoted = ApplicationRecord.connection.quote(timestamp.to_s) + "WHEN \"#{Journal.table_name}\".\"validity_period\" @> timestamp with time zone '#{comparison_time}' THEN #{quoted}" end .join(" ") end diff --git a/app/models/timestamp.rb b/app/models/timestamp.rb index b2da04db020..23b7be8810f 100644 --- a/app/models/timestamp.rb +++ b/app/models/timestamp.rb @@ -40,9 +40,9 @@ class Timestamp DATE_KEYWORD_REGEX = %r{ - ^(?:#{ALLOWED_DATE_KEYWORDS.join('|')}) # match the relative date keyword + \A(?:#{ALLOWED_DATE_KEYWORDS.join('|')}) # match the relative date keyword @(?:([0-1]?[0-9]|2[0-3]):[0-5]?[0-9]) # match the hour part - [+-](?:([0-1]?[0-9]|2[0-3]):[0-5]?[0-9])$ # match the timezone offset + [+-](?:([0-1]?[0-9]|2[0-3]):[0-5]?[0-9])\z # match the timezone offset }x def initialize(string) 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 diff --git a/spec/models/journable/historic_active_record_relation_timestamp_parsing_spec.rb b/spec/models/journable/historic_active_record_relation_timestamp_parsing_spec.rb new file mode 100644 index 00000000000..8c72508e862 --- /dev/null +++ b/spec/models/journable/historic_active_record_relation_timestamp_parsing_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe Journable::HistoricActiveRecordRelation do + let(:project) { create(:project) } + let!(:work_package) { create(:work_package, project:) } + + describe "#timestamp_case_when_statements" do + context "when a Timestamp object wraps a multi-line string" do + let(:crafted_ts) { Timestamp.new("oneDayAgo@00:00+00:00\n@' extra_content") } + let(:historic_relation) do + described_class.new(WorkPackage.all, timestamp: [crafted_ts]) + end + + it "does not allow the extra content to break out of the SQL string literal" do + sql = historic_relation.send(:timestamp_case_when_statements) + # The apostrophe in the crafted input must be SQL-escaped (doubled), not left bare. + # An unescaped @' sequence would close the string literal and allow SQL injection. + expect(sql).not_to include("@' extra_content") + end + end + + context "when a Timestamp object wraps a single-line date-keyword string" do + let(:valid_ts) { Timestamp.parse("oneDayAgo@00:00+00:00") } + let(:historic_relation) do + described_class.new(WorkPackage.all, timestamp: [valid_ts]) + end + + it "generates a WHEN/THEN clause containing the label" do + sql = historic_relation.send(:timestamp_case_when_statements) + expect(sql).to match(/WHEN .+ THEN .+oneDayAgo/) + end + end + end +end diff --git a/spec/models/timestamp_parsing_spec.rb b/spec/models/timestamp_parsing_spec.rb new file mode 100644 index 00000000000..96f4809e418 --- /dev/null +++ b/spec/models/timestamp_parsing_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +# Timestamp strings that span multiple lines must be rejected at every entry point. +# The date-keyword regex previously used ^ and $ (line anchors), which caused it to +# match only the first line of a multi-line string. The remaining lines were silently +# kept in the stored string and later interpolated verbatim into SQL. +RSpec.describe Timestamp do + let(:valid_keyword) { "oneDayAgo@00:00+00:00" } + let(:multiline_input) { "#{valid_keyword}\n@' extra_content" } + + describe ".parse" do + it "accepts a single-line date-keyword timestamp" do + expect { described_class.parse(valid_keyword) }.not_to raise_error + end + + it "rejects a multi-line string whose first line is a valid date keyword" do + expect { described_class.parse(multiline_input) }.to raise_error(ArgumentError) + end + + it "rejects a multi-line string whose first line is a valid ISO 8601 datetime" do + expect { described_class.parse("2024-01-01T00:00:00Z\nextra_content") }.to raise_error(ArgumentError) + end + end + + describe "#valid?" do + it "returns true for a single-line date-keyword timestamp" do + expect(described_class.new(valid_keyword)).to be_valid + end + + it "returns false for a multi-line string whose first line is a valid date keyword" do + expect(described_class.new(multiline_input)).not_to be_valid + end + end +end