T09 · Insecure Skill Coding Practices
Error
- Location
- stock_skill/models.py:21
- Finding
- Unsafe Joblib deserialization with bypassable path containment## Vulnerability Details **File Location**: `stock_skill/models.py:21-94` **Vulnerability Type**: Unsafe deserialization and insufficient path validation **Risk Level**: High ### Vulnerable Code ```python def _validate_model_path(file_path: str) -> None: abs_path = Path(file_path).resolve() if not str(abs_path).startswith(str(_MODELS_DIR.resolve())): raise ValueError( f"Security restriction: model files must be located under {_MODELS_DIR}." f"Attempted path: {file_path}" ) if abs_path.suffix not in _ALLOWED_EXTENSIONS: raise ValueError( f"Security restriction: extension '{abs_path.suffix}' is not allowed." ) if not abs_path.exists(): raise FileNotFoundError(f"Model file does not exist: {file_path}") ``` ```python def load_model_safe(file_path: str, loader_func=None) -> Optional[Any]: try: _validate_model_path(file_path) if not _check_file_integrity(file_path): logger.error(f"Model integrity check failed: {file_path}") return None if loader_func is None: import joblib loader_func = joblib.load model = loader_func(file_path) return model except Exception as e: logger.error(f"Model loading failed: {e}") return None ``` ### Technical Analysis `joblib.load()` uses Python pickle-compatible deserialization. Pickle data may contain reduction instructions that import modules and invoke arbitrary callables during deserialization. Loading a malicious Joblib file therefore amounts to executing code with the privileges of the running process. The `_check_file_integrity()` routine does not establish authenticity or integrity. It only limits file size and checks the filename for selected substrings. It does not compare a cryptographic digest, validate a digital signature, inspect ownership, ...[truncated 2014 chars]
- Remediation
- ## Remediation Suggestions 1. Replace the string-prefix check with a component-aware containment check: ```python trusted_root = _MODELS_DIR.resolve(strict=True) candidate = Path(file_path).resolve(strict=True) if not candidate.is_relative_to(trusted_root): raise ValueError("Model path is outside the trusted model directory") ``` 2. Verify that the candidate is a regular file and reject symbolic links or other special files where appropriate: ```python if candidate.is_symlink() or not candidate.is_file(): raise ValueError("Model must be a regular, non-symlink file") ``` 3. Avoid pickle and Joblib for files that are not guaranteed to be trusted. Prefer non-executable formats such as safetensors or a strictly validated JSON representation. 4. If Joblib must remain supported, require a cryptographic signature or an allowlisted SHA-256 digest generated through a trusted model-build process. 5. Store trusted models in a directory that is not writable by untrusted users or unrelated application components. 6. Remove `.pkl` and `.joblib` from the allowed extension list when executable serialization is unnecessary. 7. Add tests covering sibling-prefix paths, symbolic links, replaced model files, and malformed serialized content.