Skip to content

Commit c73a726

Browse files
committed
Fix for arbitrary code execution while unpickling in ipycache.load_vars() method.
Resolution for issue rossant#47 in the original repo. Any malicious command trying to process through the unpickle command would have to go through the restricted_loads() method which only allows io.StringsIO to parse. Anything else, and it would raise a UnpicklingError.
1 parent 2c334c5 commit c73a726

2 files changed

Lines changed: 44 additions & 8 deletions

File tree

ipycache.py

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,20 @@
77
# Imports
88
#------------------------------------------------------------------------------
99

10+
import hashlib
1011
# Stdlib
11-
import inspect, os, sys, textwrap, re
12+
import io
13+
import os
14+
import re
15+
import sys
1216

1317
# Our own
1418
from IPython.config.configurable import Configurable
1519
from IPython.core import magic_arguments
16-
from IPython.core.magic import Magics, magics_class, line_magic, cell_magic
17-
from IPython.utils.traitlets import Unicode
18-
from IPython.utils.io import CapturedIO, capture_output
20+
from IPython.core.magic import Magics, magics_class, cell_magic
1921
from IPython.display import clear_output
20-
import hashlib
21-
22+
from IPython.utils.io import CapturedIO
23+
from IPython.utils.traitlets import Unicode
2224

2325
#------------------------------------------------------------------------------
2426
# Six utility functions for Python 2/3 compatibility
@@ -115,6 +117,7 @@ def load_vars(path, vars):
115117
with open(path, 'rb') as f:
116118
# Load the variables from the cache.
117119
try:
120+
restricted_loads(f.read())
118121
cache = pickle.load(f)
119122
except EOFError as e:
120123
cache={}
@@ -151,8 +154,26 @@ def save_vars(path, vars_d):
151154
"""
152155
with open(path, 'wb') as f:
153156
dump(vars_d, f)
154-
155-
157+
158+
159+
# ------------------------------------------------------------------------------
160+
# RestrictedUnpickler - For mitigating arbitrary code execution while unpickling
161+
# This function provides restriction of using only the io module
162+
# ------------------------------------------------------------------------------
163+
class RestrictedUnpickler(pickle.Unpickler):
164+
165+
def find_class(self, module, name):
166+
if module == '_io' and name == 'StringIO':
167+
return getattr(sys.modules[module], name)
168+
# Forbid everything else.
169+
raise pickle.UnpicklingError("global '%s.%s' is forbidden" %
170+
(module, name))
171+
172+
173+
def restricted_loads(s):
174+
"""Helper function analogous to pickle.loads()."""
175+
return RestrictedUnpickler(io.BytesIO(s)).load()
176+
156177
#------------------------------------------------------------------------------
157178
# CapturedIO
158179
#------------------------------------------------------------------------------

test_ipycache.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,3 +206,18 @@ def ip_push(vars):
206206
ip_user_ns=user_ns, ip_run_cell=ip_run_cell, ip_push=ip_push)
207207

208208
os.remove(path)
209+
210+
211+
def test_load_exploitPickle():
212+
class vulnLoad():
213+
def __init__(self):
214+
self.a = 1
215+
216+
def __reduce__(self):
217+
return (os.system, ('uname -a',))
218+
219+
payload = vulnLoad()
220+
path = "malicious.pkl"
221+
with open("malicious.pkl", "wb") as f:
222+
pickle.dump(payload, f)
223+
assert_raises(pickle.UnpicklingError, load_vars, path, ['a'])

0 commit comments

Comments
 (0)