Merge remote-tracking branch 'origin/release/17.3' into release/17.4

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

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)

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/<uuid>/
     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}")

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

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

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