Skip to content

Commit c787557

Browse files
author
Nils Bars
committed
Add JSON-based IPC serializer replacing pickle
Introduces IPCSerializer with explicit type registration for secure deserialization. Unlike pickle, this cannot execute arbitrary code. Supports CompletedProcess, RefUtilsError types, and standard exceptions. Updates process.py to use safe_dumps/safe_loads instead of pickle, and uses config module for drop_uid/drop_gid settings.
1 parent 9f20ad2 commit c787557

3 files changed

Lines changed: 369 additions & 51 deletions

File tree

ref_utils/error.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
import signal
12
from typing import Any
3+
24
from .utils import decode_or_str
3-
import signal
5+
46

57
class RefUtilsError(Exception):
68
"""
@@ -13,7 +15,7 @@ def __init__(self, *args: str, **kwargs: Any) -> None:
1315
super().__init__(*args, **kwargs)
1416

1517
class RefUtilsProcessTimeoutError(RefUtilsError):
16-
18+
1719
def __init__(self, cmd: str, timeout: int) -> None:
1820
self.cmd: str = cmd
1921
self.timeout: int = timeout
@@ -23,7 +25,7 @@ def __str__(self) -> str:
2325
return self.msg
2426

2527
class RefUtilsProcessError(RefUtilsError):
26-
28+
2729
def __init__(self, cmd: str, exit_code: int, stdout: bytes, stderr: bytes) -> None:
2830
self.exit_code = exit_code
2931
if exit_code < 0:

ref_utils/process.py

Lines changed: 43 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,23 @@
11
"""Functions related to dropping privileges"""
2-
from multiprocessing import Pipe, Process
3-
from multiprocessing.connection import Connection
4-
from types import TracebackType
5-
from typing import Any, Callable, Dict, List, Optional, Type, Tuple, Union
6-
from functools import wraps
2+
import errno
73
import os
84
import subprocess
95
import sys
6+
import traceback
7+
import typing as t
8+
import warnings
9+
from functools import partial, wraps
10+
from multiprocessing import Pipe, Process
11+
from multiprocessing.connection import Connection
1012
from pathlib import Path
11-
import errno
12-
import pickle
13-
import io
14-
import importlib
15-
from functools import partial
13+
from types import TracebackType
14+
from typing import Any, Callable, List, Optional, Tuple, Type, Union
1615

17-
from .utils import get_user_environment, print_err, map_path_as_posix, print_ok, decode_or_str, print_warn
18-
from .error import RefUtilsError, RefUtilsProcessTimeoutError, RefUtilsProcessError
16+
from .config import get_config
17+
from .error import RefUtilsError, RefUtilsProcessError, RefUtilsProcessTimeoutError
18+
from .serialization import safe_dumps, safe_loads
19+
from .utils import decode_or_str, get_user_environment, map_path_as_posix, print_err, print_ok
1920

20-
_DEFAULT_DROP_UID = 9999
21-
_DEFAULT_DROP_GID = 9999
2221

2322
def ref_util_exception_hook(type_: Type[BaseException], value: BaseException, traceback: TracebackType, redact_traceback: bool = False) -> None:
2423
"""
@@ -47,27 +46,21 @@ def ref_util_install_global_exception_hook() -> None:
4746
hook = partial(ref_util_exception_hook, redact_traceback=False)
4847
sys.excepthook = hook
4948

50-
# Hopefully safe, if not, please tell us, dont mess with the system. Thanks :)
51-
class RestrictedUnpickler(pickle.Unpickler):
52-
ALLOWED_MODULE_NAME = {
53-
("subprocess", "CompletedProcess"),
54-
("ref_utils.error", "RefUtilsProcessError"),
55-
("ref_utils.error", "RefUtilsProcessTimeoutError"),
56-
("ref_utils.error", "RefUtilsAssertionError"),
57-
("ref_utils.error", "RefUtilsError")
58-
}
59-
60-
def find_class(self, module, name):
61-
for safe_module_name in RestrictedUnpickler.ALLOWED_MODULE_NAME:
62-
if safe_module_name == (module, name):
63-
m = importlib.import_module(module)
64-
return getattr(m, name)
65-
else:
66-
err = pickle.UnpicklingError(f"{module}.{name} is forbidden")
67-
raise RefUtilsError(f"Failed to parse the output of the target during testing. This should not happen. Please inform the staff. ({err})")
49+
# DEPRECATED: RestrictedUnpickler is kept for backward compatibility only.
50+
# New code should use safe_dumps/safe_loads from serialization module.
51+
def restricted_loads(s: bytes) -> Any:
52+
"""Deserialize data using JSON-based serialization.
6853
69-
def restricted_loads(s):
70-
return RestrictedUnpickler(io.BytesIO(s)).load()
54+
.. deprecated::
55+
Use safe_loads() from ref_utils.serialization instead.
56+
This function now uses JSON serialization internally.
57+
"""
58+
warnings.warn(
59+
"restricted_loads is deprecated, use safe_loads from ref_utils.serialization",
60+
DeprecationWarning,
61+
stacklevel=2,
62+
)
63+
return safe_loads(s)
7164

7265
def _drop_and_execute(conn: Connection, uid: int, gid: int, original_func: Callable[..., Any], *args: Any, **kwargs: Any) -> None:
7366
os.setresgid(gid, gid, gid)
@@ -76,32 +69,34 @@ def _drop_and_execute(conn: Connection, uid: int, gid: int, original_func: Calla
7669
os.setresuid(uid, uid, uid)
7770
try:
7871
ret = original_func(*args, **kwargs)
79-
pickled_ret = pickle.dumps(ret)
80-
conn.send_bytes(pickled_ret)
72+
serialized_ret = safe_dumps(ret)
73+
conn.send_bytes(serialized_ret)
74+
except AttributeError:
75+
exception_str = traceback.format_exc()
76+
print_err(f"[!] Unexpected error:\n{exception_str}")
77+
exit(1)
8178
except Exception as e:
82-
#Forward exception to our parent
83-
pickled_e = pickle.dumps(e)
84-
conn.send_bytes(pickled_e)
79+
# Forward exception to our parent
80+
serialized_e = safe_dumps(e)
81+
conn.send_bytes(serialized_e)
8582
finally:
8683
conn.close()
8784

8885
def drop_privileges(func: Callable[..., Any]) -> Callable[..., Any]:
8986
"""
9087
Decorator which drops the privileges to default UID, GID tuple before executing the decorated function.
9188
Uses fork and setuid to drop privileges.
92-
NOTE: The decorated function's output is communicated back via a pipe and encoded via pickle.
93-
Thus, we are unpickling untrusted data here!
89+
NOTE: The decorated function's output is communicated back via a pipe and encoded via JSON.
9490
"""
9591
@wraps(func)
9692
def wrapper(*args: Any, **kwargs: Any) -> Any:
9793
parent_conn, child_conn = Pipe()
98-
p = Process(target=_drop_and_execute, args=(child_conn, _DEFAULT_DROP_UID, _DEFAULT_DROP_GID, func, *args,), kwargs=kwargs)
94+
config = get_config()
95+
p = Process(target=_drop_and_execute, args=(child_conn, config.drop_uid, config.drop_gid, func, *args,), kwargs=kwargs)
9996
p.start()
100-
pickled_ret: Any = parent_conn.recv_bytes()
101-
# ! Unpickle the data that was pickled by our untrusted party in `_drop_and_execute`.
102-
# ! We are only allowing a subset of python types.
103-
# ! It would be prefereable to use JSON here to make this actually feel safe.
104-
ret = restricted_loads(pickled_ret)
97+
serialized_ret: Any = parent_conn.recv_bytes()
98+
# Deserialize using JSON-based serialization (secure alternative to pickle)
99+
ret = safe_loads(serialized_ret)
105100
p.join()
106101
if isinstance(ret, Exception):
107102
raise ret
@@ -136,7 +131,7 @@ def run(cmd_: List[Union[str, Path, bytes]], *args: str, **kwargs: Any) -> 'subp
136131
if "stdin" not in kwargs and "input" not in kwargs:
137132
kwargs["stdin"] = subprocess.DEVNULL
138133

139-
if not 'env' in kwargs:
134+
if 'env' not in kwargs:
140135
# Restore the environment from the user as of the time she called `task ...`.
141136
# NOTE: The stored environment contains user controlled input!
142137
# Never restore the environment in a privileged context.

0 commit comments

Comments
 (0)