Merge pull request #2355 from kamil-certat/fix_root_priviledges

FIX: intelmqsetup - never take ownership of /

CHANGELOG.md

@@ -41,6 +41,8 @@ CHANGELOG
 ### Packaging
 
 ### Tools
+- `intelmqsetup`:
+  - SECURITY: fixed a low-risk bug causing the tool to change owner of `/` if run with the `INTELMQ_PATHS_NO_OPT` environment variable set. This affects only the PIP package as the DEB/RPM packages don't contain this tool. (PR#2355 by Kamil Mańkowski, fixes #2354)
 
 ### Known Errors
 

intelmq/bin/intelmqsetup.py

@@ -95,6 +95,9 @@ def create_directory(directory: str, octal_mode: int):
 
 
 def change_owner(file: str, owner: Optional[str] = None, group: Optional[str] = None, log: bool = True):
+    if Path(file).as_posix() in ['/', '//']:
+        # Skip taking ownership over system root path
+        return
     if owner and Path(file).owner() != owner:
         if log:
             print(f'Fixing owner of {file!s}.')

intelmq/tests/bin/test_intelmqsetup.py

@@ -0,0 +1,35 @@
+"""Tests for the intelmqsetup
+
+SPDX-FileCopyrightText: 2023 CERT.at GmbH <https://cert.at/>
+SPDX-License-Identifier: AGPL-3.0-or-later
+"""
+
+import unittest
+from unittest import mock
+
+from intelmq.bin import intelmqsetup
+
+
+class TestOwnership(unittest.TestCase):
+    @mock.patch("shutil.chown")
+    def test_skip_changing_root_path_ownership(self, chown_mock):
+        with mock.patch.object(intelmqsetup.Path, 'owner') as owner_mock:
+            with mock.patch.object(intelmqsetup.Path, 'group') as group_mock:
+                owner_mock.return_value = 'root'
+                group_mock.return_value = 'root'
+                intelmqsetup.change_owner('/', 'intelmq', 'intelmq')
+                intelmqsetup.change_owner('//', 'intelmq', 'intelmq')
+                intelmqsetup.change_owner('///', 'intelmq', 'intelmq')
+
+        chown_mock.assert_not_called()
+
+    @mock.patch("shutil.chown")
+    def test_change_file_ownership(self, chown_mock):
+        with mock.patch.object(intelmqsetup.Path, 'owner') as owner_mock:
+            with mock.patch.object(intelmqsetup.Path, 'group') as group_mock:
+                owner_mock.return_value = 'root'
+                group_mock.return_value = 'root'
+                intelmqsetup.change_owner('/the/path', 'intelmq', 'intelmq')
+
+        chown_mock.assert_any_call('/the/path', user='intelmq')
+        chown_mock.assert_any_call('/the/path', group='intelmq')