From 93a4f8abe32de11f2b65902cf07c097d69d4880c Mon Sep 17 00:00:00 2001
From: Jens Kraemer <jk@jkraemer.net>
Date: Fri, 17 Apr 2026 12:46:22 +0800
Subject: [PATCH] tighten SVN repository URL validation

- the URL will be escaped / quoted downstream so not really an actual security
  issue, but still, I do not see a reason to allow values with multiple lines here
---
 app/models/repository/subversion.rb           |  2 +-
 .../repositories_controller_test.rb           | 25 +++++++++++++++++++
 test/unit/repository_subversion_test.rb       |  9 +++++--
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/app/models/repository/subversion.rb b/app/models/repository/subversion.rb
index 030bdb46f..324e7da8d 100644
--- a/app/models/repository/subversion.rb
+++ b/app/models/repository/subversion.rb
@@ -21,7 +21,7 @@ require 'redmine/scm/adapters/subversion_adapter'
 
 class Repository::Subversion < Repository
   validates_presence_of :url
-  validates_format_of :url, :with => %r{\A(http|https|svn(\+[^\s:\/\\]+)?|file):\/\/.+}i
+  validates_format_of :url, :with => %r{\A(http|https|svn(\+[^\s:\/\\]+)?|file):\/\/.+\z}i
 
   def self.scm_adapter_class
     Redmine::Scm::Adapters::SubversionAdapter
diff --git a/test/functional/repositories_controller_test.rb b/test/functional/repositories_controller_test.rb
index 8f32d1a28..ccd6606d5 100644
--- a/test/functional/repositories_controller_test.rb
+++ b/test/functional/repositories_controller_test.rb
@@ -120,6 +120,31 @@ class RepositoriesControllerTest < Redmine::RepositoryControllerTest
     end
   end
 
+  def test_create_should_reject_subversion_url_with_newline_injection
+    @request.session[:user_id] = 1
+    [
+      "file:///test\nfoo",
+      "svn+ssh://example.com/repo\r\nbar"
+    ].each do |injected_url|
+      assert_no_difference 'Repository.count', "expected #{injected_url.inspect} to be rejected" do
+        post(
+          :create,
+          :params => {
+            :project_id => 'subproject1',
+            :repository_scm => 'Subversion',
+            :repository => {
+              :url => injected_url,
+              :is_default => '1',
+              :identifier => ''
+            }
+          }
+        )
+      end
+      assert_response :success
+      assert_select_error /URL is invalid/
+    end
+  end
+
   def test_edit
     @request.session[:user_id] = 1
     get(:edit, :params => {:id => 11})
diff --git a/test/unit/repository_subversion_test.rb b/test/unit/repository_subversion_test.rb
index dfdf520e7..3d47007ae 100644
--- a/test/unit/repository_subversion_test.rb
+++ b/test/unit/repository_subversion_test.rb
@@ -35,14 +35,19 @@ class RepositorySubversionTest < ActiveSupport::TestCase
 
   def test_invalid_url
     set_language_if_valid 'en'
-    ['invalid', 'http://', 'svn://', 'svn+ssh://', 'file://'].each do |url|
+    invalid_urls = [
+      'invalid', 'http://', 'svn://', 'svn+ssh://', 'file://',
+      "http://valid\nfoo",
+      "svn://valid\r\nbar"
+    ]
+    invalid_urls.each do |url|
       repo =
         Repository::Subversion.new(
           :project      => @project,
           :identifier   => 'test',
           :url => url
         )
-      assert !repo.save
+      assert !repo.save, "expected #{url.inspect} to be rejected"
       assert_equal ["is invalid"], repo.errors[:url]
     end
   end
-- 
2.53.0