Skip to content

Commit 6247663

Browse files
authored
Merge pull request from GHSA-p867-fxfr-ph2w
* Fix setting permissions for local sqlite database Thanks to Jan Schejbal for responsible disclosure! There used to be a brief moment between creation of the sqlite database and applying chmod, now there is no such delay. * Rename self.home to self.test_home in tests * Actually call the test cleanup * Add a test for sqlite permissions * Skip the sqlite chmod test on Windows
1 parent 146aaa6 commit 6247663

File tree

3 files changed

+39
-19
lines changed

3 files changed

+39
-19
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313

1414
### Fixed
1515
* Fix downloading files with unverified checksum
16+
* Fix setting permissions for local sqlite database (thanks to Jan Schejbal for responsible disclosure!)
1617

1718
## [1.14.0] - 2021-12-23
1819

b2sdk/account_info/sqlite_account_info.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,16 @@ def _connect(self):
165165

166166
def _create_database(self, last_upgrade_to_run):
167167
"""
168-
Make sure that the database is created and sets the file permissions.
168+
Make sure that the database is created and has appropriate file permissions.
169169
This should be done before storing any sensitive data in it.
170170
"""
171+
# Prepare a file
172+
fd = os.open(
173+
self.filename,
174+
flags=os.O_RDWR | os.O_CREAT,
175+
mode=stat.S_IRUSR | stat.S_IWUSR,
176+
)
177+
os.close(fd)
171178
# Create the tables in the database
172179
conn = self._connect()
173180
try:
@@ -176,9 +183,6 @@ def _create_database(self, last_upgrade_to_run):
176183
finally:
177184
conn.close()
178185

179-
# Set the file permissions
180-
os.chmod(self.filename, stat.S_IRUSR | stat.S_IWUSR)
181-
182186
def _create_tables(self, conn, last_upgrade_to_run):
183187
conn.execute(
184188
"""

test/unit/account_info/test_account_info.py

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import os
1515
import platform
1616
import shutil
17+
import stat
1718
import tempfile
1819

1920
import pytest
@@ -319,15 +320,29 @@ def setUp(self, request):
319320
os.unlink(self.db_path)
320321
except OSError:
321322
pass
322-
self.home = tempfile.mkdtemp()
323+
self.test_home = tempfile.mkdtemp()
323324

324325
yield
325-
for cleanup_method in [lambda: os.unlink(self.db_path), lambda: shutil.rmtree(self.home)]:
326+
for cleanup_method in [
327+
lambda: os.unlink(self.db_path), lambda: shutil.rmtree(self.test_home)
328+
]:
326329
try:
327-
cleanup_method
330+
cleanup_method()
328331
except OSError:
329332
pass
330333

334+
@pytest.mark.skipif(
335+
platform.system() == 'Windows',
336+
reason='different permission system on Windows'
337+
)
338+
def test_permissions(self):
339+
"""
340+
Test that a new database won't be readable by just any user
341+
"""
342+
s = SqliteAccountInfo(file_name=self.db_path,)
343+
mode = os.stat(self.db_path).st_mode
344+
assert stat.filemode(mode) == '-rw-------'
345+
331346
def test_corrupted(self):
332347
"""
333348
Test that a corrupted file will be replaced with a blank file.
@@ -371,7 +386,7 @@ def _make_sqlite_account_info(self, env=None, last_upgrade_to_run=None):
371386
:param dict env: Override Environment variables.
372387
"""
373388
# Override HOME to ensure hermetic tests
374-
with mock.patch('os.environ', env or {'HOME': self.home}):
389+
with mock.patch('os.environ', env or {'HOME': self.test_home}):
375390
return SqliteAccountInfo(
376391
file_name=self.db_path if not env else None,
377392
last_upgrade_to_run=last_upgrade_to_run,
@@ -380,24 +395,24 @@ def _make_sqlite_account_info(self, env=None, last_upgrade_to_run=None):
380395
def test_uses_default(self):
381396
account_info = self._make_sqlite_account_info(
382397
env={
383-
'HOME': self.home,
384-
'USERPROFILE': self.home,
398+
'HOME': self.test_home,
399+
'USERPROFILE': self.test_home,
385400
}
386401
)
387402
actual_path = os.path.abspath(account_info.filename)
388-
assert os.path.join(self.home, '.b2_account_info') == actual_path
403+
assert os.path.join(self.test_home, '.b2_account_info') == actual_path
389404

390405
def test_uses_xdg_config_home(self, apiver):
391406
with WindowsSafeTempDir() as d:
392407
account_info = self._make_sqlite_account_info(
393408
env={
394-
'HOME': self.home,
395-
'USERPROFILE': self.home,
409+
'HOME': self.test_home,
410+
'USERPROFILE': self.test_home,
396411
XDG_CONFIG_HOME_ENV_VAR: d,
397412
}
398413
)
399414
if apiver in ['v0', 'v1']:
400-
expected_path = os.path.abspath(os.path.join(self.home, '.b2_account_info'))
415+
expected_path = os.path.abspath(os.path.join(self.test_home, '.b2_account_info'))
401416
else:
402417
assert os.path.exists(os.path.join(d, 'b2'))
403418
expected_path = os.path.abspath(os.path.join(d, 'b2', 'account_info'))
@@ -406,12 +421,12 @@ def test_uses_xdg_config_home(self, apiver):
406421

407422
def test_uses_existing_file_and_ignores_xdg(self):
408423
with WindowsSafeTempDir() as d:
409-
default_db_file_location = os.path.join(self.home, '.b2_account_info')
424+
default_db_file_location = os.path.join(self.test_home, '.b2_account_info')
410425
open(default_db_file_location, 'a').close()
411426
account_info = self._make_sqlite_account_info(
412427
env={
413-
'HOME': self.home,
414-
'USERPROFILE': self.home,
428+
'HOME': self.test_home,
429+
'USERPROFILE': self.test_home,
415430
XDG_CONFIG_HOME_ENV_VAR: d,
416431
}
417432
)
@@ -423,8 +438,8 @@ def test_account_info_env_var_overrides_xdg_config_home(self):
423438
with WindowsSafeTempDir() as d:
424439
account_info = self._make_sqlite_account_info(
425440
env={
426-
'HOME': self.home,
427-
'USERPROFILE': self.home,
441+
'HOME': self.test_home,
442+
'USERPROFILE': self.test_home,
428443
XDG_CONFIG_HOME_ENV_VAR: d,
429444
B2_ACCOUNT_INFO_ENV_VAR: os.path.join(d, 'b2_account_info'),
430445
}

0 commit comments

Comments
 (0)