Add patch to fix code injection vulnerability (Closes: #1004194)
Nilesh Patra
2 years ago
| 0 | From 4b0070a4f30cbf6d5e12e6274b242b62ea11c81b Mon Sep 17 00:00:00 2001 | |
| 1 | From: Delgan <delgan.py@gmail.com> | |
| 2 | Date: Fri, 21 Jan 2022 11:21:58 +0100 | |
| 3 | Subject: [PATCH] Remove use of "pickle.loads()" to comply with security tools | |
| 4 | (#563) | |
| 5 | ||
| 6 | --- | |
| 7 | loguru/_recattrs.py | 18 +++++++----------- | |
| 8 | tests/test_add_option_enqueue.py | 28 +++++++++++++++++++++++++--- | |
| 9 | 2 files changed, 32 insertions(+), 14 deletions(-) | |
| 10 | ||
| 11 | --- a/loguru/_recattrs.py | |
| 12 | +++ b/loguru/_recattrs.py | |
| 13 | @@ -64,18 +64,14 @@ | |
| 14 | return "(type=%r, value=%r, traceback=%r)" % (self.type, self.value, self.traceback) | |
| 15 | ||
| 16 | def __reduce__(self): | |
| 17 | + # The traceback is not picklable so we need to remove it. Also, some custom exception | |
| 18 | + # values aren't picklable either. For user convenience, we try first to serialize it and | |
| 19 | + # we remove the value in case or error. As an optimization, we could have re-used the | |
| 20 | + # dumped value during unpickling, but this requires using "pickle.loads()" which is | |
| 21 | + # flagged as insecure by some security tools. | |
| 22 | try: | |
| 23 | - pickled_value = pickle.dumps(self.value) | |
| 24 | + pickle.dumps(self.value) | |
| 25 | except pickle.PickleError: | |
| 26 | return (RecordException, (self.type, None, None)) | |
| 27 | else: | |
| 28 | - return (RecordException._from_pickled_value, (self.type, pickled_value, None)) | |
| 29 | - | |
| 30 | - @classmethod | |
| 31 | - def _from_pickled_value(cls, type_, pickled_value, traceback_): | |
| 32 | - try: | |
| 33 | - value = pickle.loads(pickled_value) | |
| 34 | - except pickle.PickleError: | |
| 35 | - return cls(type_, None, traceback_) | |
| 36 | - else: | |
| 37 | - return cls(type_, value, traceback_) | |
| 38 | + return (RecordException, (self.type, self.value, None)) | |
| 39 | --- a/tests/test_add_option_enqueue.py | |
| 40 | +++ b/tests/test_add_option_enqueue.py | |
| 41 | @@ -212,8 +212,7 @@ | |
| 42 | assert err == "".join("%d\n" % i for i in range(10)) | |
| 43 | ||
| 44 | ||
| 45 | -@pytest.mark.parametrize("arg", [NotPicklable(), NotUnpicklable()]) | |
| 46 | -def test_logging_not_picklable_exception(arg): | |
| 47 | +def test_logging_not_picklable_exception(): | |
| 48 | exception = None | |
| 49 | ||
| 50 | def sink(message): | |
| 51 | @@ -223,7 +222,30 @@ | |
| 52 | logger.add(sink, enqueue=True, catch=False) | |
| 53 | ||
| 54 | try: | |
| 55 | - raise ValueError(arg) | |
| 56 | + raise ValueError(NotPicklable()) | |
| 57 | + except Exception: | |
| 58 | + logger.exception("Oups") | |
| 59 | + | |
| 60 | + logger.remove() | |
| 61 | + | |
| 62 | + type_, value, traceback_ = exception | |
| 63 | + assert type_ is ValueError | |
| 64 | + assert value is None | |
| 65 | + assert traceback_ is None | |
| 66 | + | |
| 67 | + | |
| 68 | +@pytest.mark.xfail(reason="No way to safely deserialize exception yet") | |
| 69 | +def test_logging_not_unpicklable_exception(): | |
| 70 | + exception = None | |
| 71 | + | |
| 72 | + def sink(message): | |
| 73 | + nonlocal exception | |
| 74 | + exception = message.record["exception"] | |
| 75 | + | |
| 76 | + logger.add(sink, enqueue=True, catch=False) | |
| 77 | + | |
| 78 | + try: | |
| 79 | + raise ValueError(NotUnpicklable()) | |
| 80 | except Exception: | |
| 81 | logger.exception("Oups") | |
| 82 |