Project

General

Profile

Actions
Copy link

Defect #39862

closed

Attachments functionality for (custom) plugins broken since fix for CVE-2022-44030

Added by Naha Sapimapethilon about 1 year ago. Updated 10 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Marius BĂLTEANU
Category:
Plugin API
Target version:
5.0.8
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:
5.0.0

Description

I notice this in current 5.1-stable branch, but should be all the way back to defect #37772 if I tracked it right.

The problem is with the new constraints for some attachments routes, when used by a plugin. My plugin makes use of acts_as_attachable in its model and :partial=>'attachments/form' in its view, just like described here.

This is now broken with an error from app/helpers/attachments_helper.rb:23:in `container_attachments_edit_path':

No route matches {:action=>"edit_all", :controller=>"attachments", :id=>"138026", 
 :object_id=>138026, :object_type=>"myplugin",
 :project_id=>"1"}, possible unmatched constraints: [:object_type]

its actually coming from this block in config/routes.rb (finding that took me a while):

constraints object_type: /(issues|versions|news|messages|wiki_pages|projects|documents|journals)/ do
   get 'attachments/:object_type/:object_id/edit', :to => 'attachments#edit_all', :as => :object_attachments_edit
   patch 'attachments/:object_type/:object_id', :to => 'attachments#update_all', :as => :object_attachments
   get 'attachments/:object_type/:object_id/download', :to => 'attachments#download_all', :as => :object_attachments_download
end

the list of constraints on object_type needs myplugin in it, so it gets permitted to use these routes.

Since plugin routes get loaded at the very end of config/routes.rb I can't just overwrite/redefine since it already exists at the time I get loaded. Also I spot no functionality in the routing code of rails that allows modification from within an included routes file or at runtime via Rails.application.routes.routes... looks all read-only.

My workaround so far is to modify the release by

sed -i config/routes.rb -e '/constraints object_type:/ s/documents|journals/documents|journals|myplugin/'

right before starting up Redmine.

I think a proper solution would be to have this list be expandable somehow, perhaps via myplugin/init.rb?

Am a little lost here solving it on my own.

Files

0001-dynamic-object_type-routing-constraint-39862.patch (3.83 KB) 0001-dynamic-object_type-routing-constraint-39862.patch Jens Krämer, 2023-12-18 06:40

Related issues

Precedes Redmine - Feature #39948: Add Redmine::Plugin proxy method for Redmine::Acts::Attachable::ObjectTypeConstraint.register_object_typeClosedMarius BĂLTEANU

Actions
Actions
Copy link
 #1

Updated by Jens Krämer about 1 year ago

One way to fix this would be with a dynamic routing constraint which can be modified by plugins as in the attached patch

Actions
Copy link
 #2

Updated by Naha Sapimapethilon about 1 year ago

that patch did not apply to my 5.1-stable branch. However, I finagled the changes into config/routes.rb and acts_as_attachable.rb and that works.

also I am fine with myplugin/init.rb having

Redmine::Plugin.register :myplugin do
  ... 
  Redmine::Acts::Attachable::ObjectTypeConstraint.register_object_type(Expense.name.underscore.pluralize) 
  ...
end

thanks!

Actions
Copy link
 #3

Updated by Mischa The Evil about 1 year ago

  • Target version set to 5.0.8
  • Affected version changed from 5.1.1 to 5.0.0
Actions
Copy link
 #4

Updated by Marius BĂLTEANU about 1 year ago

  • Status changed from New to Resolved
  • Assignee set to Marius BĂLTEANU
  • Resolution set to Fixed

Committed the fix on trunk and backported it to stable branches. On 5.0-stable, the fix is without the test because the plugins routing test was introduced in 5.1.

Actions
Copy link
 #5

Updated by Alexander Meindl about 1 year ago

Hi,

this change breaks all redmineup plugins with Redmine 5.1-stable branch (I suppose same with 5.0-stable branch). They use redmine_crm gem in all plugins with this compatibility issue.

bundle exec rake db:migrate
rake aborted!
NoMethodError: undefined method `[]' for Redmine::Acts::Attachable::ObjectTypeConstraint:Class

          options[:object_type] = /.+/ if options[:object_type] && options[:object_type].is_a?(Regexp)
                                                 ^^^^^^^^^^^^^^
/srv/redmine/.local/share/gem/ruby/3.1.0/gems/redmine_crm-0.0.62/lib/redmine_crm/compatibility/routing_mapper_patch.rb:15:in `constraints_with_redmine_crm'
/srv/redmine/redmine/config/routes.rb:322:in `block in <top (required)>'
/srv/redmine/.local/share/gem/ruby/3.1.0/gems/actionpack-6.1.7.6/lib/action_dispatch/routing/route_set.rb:427:in `instance_exec'
/srv/redmine/.local/share/gem/ruby/3.1.0/gems/actionpack-6.1.7.6/lib/action_dispatch/routing/route_set.rb:427:in `eval_block'
/srv/redmine/.local/share/gem/ruby/3.1.0/gems/actionpack-6.1.7.6/lib/action_dispatch/routing/route_set.rb:409:in `draw'
/srv/redmine/redmine/config/routes.rb:20:in `<top (required)>'

I am not sure this change should go to stable branch, if it breaks existing plugins.

Actions
Copy link
 #6

Updated by Marius BĂLTEANU about 1 year ago

Alexander Meindl wrote in #note-5:

Hi,

this change breaks all redmineup plugins with Redmine 5.1-stable branch (I suppose same with 5.0-stable branch). They use redmine_crm gem in all plugins with this compatibility issue.

[...]

I am not sure this change should go to stable branch, if it breaks existing plugins.

In this case, yes, I think we cannot deliver the change in maintenance releases. Jens, Mischa, what do you think?

Actions
Copy link
 #7

Updated by ashraf alzyoud about 1 year ago

All redmineup plugins broken after update

Actions
Copy link
 #9

Updated by Holger Just about 1 year ago

Well, the patch in the redmine_crm gem (here in version 0.0.62 licensed under GPL 2) is as follows:

# lib/redmine_crm/compatibility/routing_mapper_patch.rb

module RedmineCrm
  module Patches
    module RoutingMapperPatch
      def self.included(base)
        base.send(:include, InstanceMethods)

        base.class_eval do
          alias_method :constraints_without_redmine_crm, :constraints
          alias_method :constraints, :constraints_with_redmine_crm
        end
      end

      module InstanceMethods
        def constraints_with_redmine_crm(options = {}, &block)
          options[:object_type] = /.+/ if options[:object_type] && options[:object_type].is_a?(Regexp)
          constraints_without_redmine_crm(options, &block)
        end
      end
    end
  end
end

unless ActionDispatch::Routing::Mapper.included_modules.include?(RedmineCrm::Patches::RoutingMapperPatch)
  ActionDispatch::Routing::Mapper.send(:include, RedmineCrm::Patches::RoutingMapperPatch)
end

This patch thus circumvents/removes one of the security fixes we have introduced in #37772 for CVE-2022-44030. The patch in the plugin gem thus reduces the security of the entire Redmine the plugins are installed to.

As they do not use a supported Redmine extension API but effectively change code on assumptions they made at the time they created the patch, this was always prone to breakage in any release.

Accordingly, rather than postpone the patch in this issue here, we should roll it out in order to give the plugin authors a supported extension API so that they can fix their plugin in a way which does not impact Redmine's security and is not prone to arbitrary breakage anymore. The underlying issue must be fixed by the plugin authors in any case.

Actions
Copy link
 #10

Updated by Marius BĂLTEANU about 1 year ago

  • Status changed from Resolved to Closed

Thanks Holger, I agree with your arguments. Closing this.

Actions
Copy link
 #11

Updated by Kirill Bezrukov (RedmineUP) about 1 year ago

Thank you Holger.

We just updated our gem redmine_crm v0.0.63 with routes fix

Holger Just wrote in #note-9:

Well, the patch in the redmine_crm gem (here in version 0.0.62 licensed under GPL 2) is as follows:

Actions
Copy link
 #12

Updated by Mischa The Evil about 1 year ago

  • Precedes Feature #39948: Add Redmine::Plugin proxy method for Redmine::Acts::Attachable::ObjectTypeConstraint.register_object_type added
Actions
Copy link
 #13

Updated by Go MAEDA about 1 year ago

  • Status changed from Closed to Reopened

The method Redmine::Acts::Attachable::ObjectTypeConstraint.param_expression, introduced in r22551, is causing an exception when run with Ruby 2.7. This is due to the Set class in Ruby 2.7 not having a join method.

You can observe the error by running bin/rails test test/integration/attachments_test.rb.

Failure:
AttachmentsTest#test_download_all_with_wrong_container_type [/Users/maeda/redmines/trunk/test/integration/attachments_test.rb:242]:
Expected response to be a <404: missing>, but was a <500: Internal Server Error>.
Expected: 404
  Actual: 500

This can be fixed by converting the Set object to an array before applying the join method. The following patch should resolve the issue:

diff --git a/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb b/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb
index 9c09a7870..43efe8cd3 100644
--- a/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb
+++ b/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb
@@ -39,7 +39,7 @@ module Redmine
           end

           def param_expression
-            @param_expression ||= Regexp.new("^(#{object_types.join("|")})$")
+            @param_expression ||= Regexp.new("^(#{object_types.to_a.join("|")})$")
           end
         end
       end
Actions
Copy link
 #14

Updated by Marius BĂLTEANU about 1 year ago

  • Status changed from Reopened to Closed

Committed and merged to stable branches, thanks!

Actions
Copy link
 #15

Updated by f0x Autumn 10 months ago

Alexander Meindl wrote in #note-5:

Hi,

this change breaks all redmineup plugins with Redmine 5.1-stable branch (I suppose same with 5.0-stable branch). They use redmine_crm gem in all plugins with this compatibility issue.

[...]

I am not sure this change should go to stable branch, if it breaks existing plugins.

Marius BĂLTEANU wrote in #note-14:

Committed and merged to stable branches, thanks!

It was in Redmine 5.0.8

But I got the same error in the 5.1.2 ( #40409 )

Actions
Copy link

Also available in: Atom PDF