diff --git a/app/assets/javascripts/app/views/package.jst.eco b/app/assets/javascripts/app/views/package.jst.eco
index c7c34023e..cb2a519ab 100644
--- a/app/assets/javascripts/app/views/package.jst.eco
+++ b/app/assets/javascripts/app/views/package.jst.eco
@@ -3,6 +3,11 @@
+
+ <%- @T('The installation of packages comes with security implications, because arbitrary code will be executed in the context of the Zammad application.') %>
+
+ <%- @T('Only packages from known, trusted and verfied sources should be installed.') %>
+
<%- @T('After installing, updating or uninstalling packages the following commands need to be executed on the server:') %>
@@ -48,4 +53,4 @@
<% end %>
-
\ No newline at end of file
+
diff --git a/app/models/package.rb b/app/models/package.rb
index 22bd30d64..b81631b57 100644
--- a/app/models/package.rb
+++ b/app/models/package.rb
@@ -263,32 +263,37 @@ subsequently in a separate step.
)
end
- # store package
- if !data[:reinstall]
- package_db = Package.create(meta)
- Store.add(
- object: 'Package',
- o_id: package_db.id,
- data: package.to_json,
- filename: "#{meta[:name]}-#{meta[:version]}.zpm",
- preferences: {},
- created_by_id: UserInfo.current_user_id || 1,
- )
- end
+ Transaction.execute do
+ # store package
+ if !data[:reinstall]
+ package_db = Package.create(meta)
+ Store.add(
+ object: 'Package',
+ o_id: package_db.id,
+ data: package.to_json,
+ filename: "#{meta[:name]}-#{meta[:version]}.zpm",
+ preferences: {},
+ created_by_id: UserInfo.current_user_id || 1,
+ )
+ end
- # write files
- package['files'].each do |file|
- permission = file['permission'] || '644'
- content = Base64.decode64(file['content'])
- _write_file(file['location'], permission, content)
- end
+ # write files
+ package['files'].each do |file|
+ if !allowed_file_path?(file['location'])
+ raise "Can't create file, because of not allowed file location: #{file['location']}!"
+ end
- # update package state
- package_db.state = 'installed'
- package_db.save
+ permission = file['permission'] || '644'
+ content = Base64.decode64(file['content'])
+ _write_file(file['location'], permission, content)
+ end
+
+ # update package state
+ package_db.state = 'installed'
+ package_db.save
+ end
# prebuild assets
-
package_db
end
@@ -483,4 +488,9 @@ execute all pending package migrations at once
true
end
+
+ def self.allowed_file_path?(file)
+ file.exclude?('..') && file.exclude?('%2e%2e')
+ end
+ private_class_method :allowed_file_path?
end
diff --git a/spec/models/package_spec.rb b/spec/models/package_spec.rb
new file mode 100644
index 000000000..87eb74829
--- /dev/null
+++ b/spec/models/package_spec.rb
@@ -0,0 +1,82 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe Package, type: :model do
+ let(:package_zpm_files_json) do
+ <<-JSON
+ [
+ {
+ "permission": "644",
+ "location": "example.rb",
+ "content": "YWJjw6TDtsO8w58="
+ },
+ {
+ "permission": "644",
+ "location": "app/controllers/test_controller.rb",
+ "content": "YWJjw6TDtsO8w58="
+ }
+ ]
+ JSON
+ end
+ let(:package_zpm_json) do
+ <<-JSON
+ {
+ "name": "UnitTestSample",
+ "version": "1.0.1",
+ "vendor": "Zammad Foundation",
+ "license": "ABC",
+ "url": "https://zammad.org/",
+ "description": [
+ {
+ "language": "en",
+ "text": "some description"
+ }
+ ],
+ "files": #{package_zpm_files_json}
+ }
+ JSON
+ end
+
+ context 'with different file locations' do
+ context 'with correct file locations' do
+ it 'installation should work' do
+ expect(described_class.install(string: package_zpm_json)).to be_truthy
+ end
+ end
+
+ shared_examples 'check not allowed file location' do |file_location|
+ let(:package_zpm_files_json) do
+ <<-JSON
+ [
+ {
+ "permission": "644",
+ "location": "example.rb",
+ "content": "YWJjw6TDtsO8w58="
+ },
+ {
+ "permission": "644",
+ "location": "#{file_location}",
+ "content": "YWJjw6TDtsO8w58="
+ }
+ ]
+ JSON
+ end
+
+ it 'installation should raise a error and package/store should not be present, because of not allowed file location' do
+ expect { described_class.install(string: package_zpm_json) }
+ .to raise_error(RuntimeError)
+ .and change(described_class, :count).by(0)
+ .and change(Store, :count).by(0)
+ end
+ end
+
+ context "with not allowed file location part: '..'" do
+ include_examples 'check not allowed file location', '../../../../../tmp/test_controller.rb'
+ end
+
+ context "with not allowed file location part: '%2e%2e'" do
+ include_examples 'check not allowed file location', '%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/tmp/test_controller.rb'
+ end
+ end
+end