CVE-2023-32303: Incorrect Permission Assignment for Critical Resource (CWE-732)
value: Planet is software that provides satellite data. The secret file stores the user's Planet API authentication information. It should only be accessible by the user, but before version 2.0.1, its permissions allowed the user's group and non-group to read the file as well. This issue was patched in version 2.0.1. As a workaround, set the secret file permissions to only user read/write by hand.

CWE description: The product specifies permissions for a security-critical resource in a way that allows that resource to be read or modified by unintended actors.
CVSS v3 base score: 5.5
Published: 2023-05-12T21:15Z
Repository: https://github.com/planetlabs/planet-client-python
Language: Python
Files changed: planet/auth.py, tests/unit/test_auth.py
Commit date: 2023-05-10 13:14:06 -0700
Commit message: enforce restricting secret file permissions to user read/write

Diff: diff --git a/planet/auth.py b/planet/auth.py
index e50f1af..d43e84f 100644
--- a/planet/auth.py
+++ b/planet/auth.py
@@ -17,19 +17,20 @@ from __future__ import annotations  # https://stackoverflow.com/a/33533514
 import abc
 import json
 import logging
 import os
 import pathlib
+import stat
 import typing
+from typing import Optional
 
 import httpx
 import jwt
 
 from . import http
 from .constants import ENV_API_KEY, PLANET_BASE_URL, SECRET_FILE_PATH
 from .exceptions import AuthException
-from typing import Optional
 
 LOGGER = logging.getLogger(__name__)
 
 BASE_URL = f'{PLANET_BASE_URL}/v0/auth'
 
@@ -224,12 +225,19 @@ class APIKeyAuth(httpx.BasicAuth, Auth):
         return self._key
 
 
 class _SecretFile:
 
-    def __init__(self, path):
-        self.path = path
+    def __init__(self, path: typing.Union[str, pathlib.Path]):
+        self.path = pathlib.Path(path)
+
+        self.permissions = stat.S_IRUSR | stat.S_IWUSR  # user rw
+
+        # in sdk versions <=2.0.0, secret file was created with the wrong
+        # permissions, fix this automatically as well as catching the unlikely
+        # cases where the permissions get changed externally
+        self._enforce_permissions()
 
     def write(self, contents: dict):
         try:
             secrets_to_write = self.read()
             secrets_to_write.update(contents)
@@ -238,13 +246,31 @@ class _SecretFile:
 
         self._write(secrets_to_write)
 
     def _write(self, contents: dict):
         LOGGER.debug(f'Writing to {self.path}')
-        with open(self.path, 'w') as fp:
+
+        def opener(path, flags):
+            return os.open(path, flags, self.permissions)
+
+        with open(self.path, 'w', opener=opener) as fp:
             fp.write(json.dumps(contents))
 
     def read(self) -> dict:
         LOGGER.debug(f'Reading from {self.path}')
         with open(self.path, 'r') as fp:
             contents = json.loads(fp.read())
         return contents
+
+    def _enforce_permissions(self):
+        '''if the file's permissions are not what they should be, fix them'''
+        try:
+            # in octal, permissions is the last three bits of the mode
+            file_permissions = self.path.stat().st_mode & 0o777
+            if file_permissions != self.permissions:
+                LOGGER.debug(
+                    f'{self.path} permissions are {oct(file_permissions)}, '
+                    f'should be {oct(self.permissions)}. Fixing.')
+                self.path.chmod(self.permissions)
+        except FileNotFoundError:
+            # just skip it if the secret file doesn't exist
+            pass
diff --git a/tests/unit/test_auth.py b/tests/unit/test_auth.py
index 511b516..a999ebe 100644
--- a/tests/unit/test_auth.py
+++ b/tests/unit/test_auth.py
@@ -29,11 +29,11 @@ def test_secretfile_read():
     return
 
 
 @pytest.fixture
 def secret_path(monkeypatch, tmp_path):
-    secret_path = str(tmp_path / '.test')
+    secret_path = tmp_path / '.test'
     monkeypatch.setattr(auth, 'SECRET_FILE_PATH', secret_path)
     yield secret_path
 
 
 def test_Auth_from_key():
@@ -136,5 +136,21 @@ def test_Auth_store_exists(tmp_path):
     test_auth = auth.Auth.from_key('test')
     test_auth.store(secret_path)
 
     with open(secret_path, 'r') as fp:
         assert json.loads(fp.read()) == {"key": "test", "existing": "exists"}
+
+
+def test__SecretFile_permissions_doesnotexist(secret_path):
+    '''No exception is raised if the file doesn't exist'''
+    auth._SecretFile(secret_path)
+
+
+def test__SecretFile_permissions_incorrect(secret_path):
+    '''Incorrect permissions are fixed'''
+    with open(secret_path, 'w') as fp:
+        fp.write('{"existing": "exists"}')
+
+    secret_path.chmod(0o666)
+
+    auth._SecretFile(secret_path)
+    assert secret_path.stat().st_mode & 0o777 == 0o600

Vulnerable code: from typing import Optional
    def __init__(self, path):
        self.path = path
        with open(self.path, 'w') as fp:
    secret_path = str(tmp_path / '.test')
Fixed code: import stat
from typing import Optional
    def __init__(self, path: typing.Union[str, pathlib.Path]):
        self.path = pathlib.Path(path)

        self.permissions = stat.S_IRUSR | stat.S_IWUSR  # user rw

        # in sdk versions <=2.0.0, secret file was created with the wrong
        # permissions, fix this automatically as well as catching the unlikely
        # cases where the permissions get changed externally
        self._enforce_permissions()

        def opener(path, flags):
            return os.open(path, flags, self.permissions)

        with open(self.path, 'w', opener=opener) as fp:

    def _enforce_permissions(self):
        '''if the file's permissions are not what they should be, fix them'''
        try:
            # in octal, permissions is the last three bits of the mode
            file_permissions = self.path.stat().st_mode & 0o777
            if file_permissions != self.permissions:
                LOGGER.debug(
                    f'{self.path} permissions are {oct(file_permissions)}, '
                    f'should be {oct(self.permissions)}. Fixing.')
                self.path.chmod(self.permissions)
        except FileNotFoundError:
            # just skip it if the secret file doesn't exist
            pass
    secret_path = tmp_path / '.test'


def test__SecretFile_permissions_doesnotexist(secret_path):
    '''No exception is raised if the file doesn't exist'''
    auth._SecretFile(secret_path)


def test__SecretFile_permissions_incorrect(secret_path):
    '''Incorrect permissions are fixed'''
    with open(secret_path, 'w') as fp:
        fp.write('{"existing": "exists"}')

    secret_path.chmod(0o666)

    auth._SecretFile(secret_path)
    assert secret_path.stat().st_mode & 0o777 == 0o600