🛠 Recurring Bug Patterns & Analysis
🐞 Bug List
| ID | Description | Severity | Location | Status |
|---|---|---|---|---|
| BUG-001 | Improper control of generation of code (Code Injection) |
Critical | controllers/front/GetFileController.php:309 |
Fixed |
| BUG-002 | Improper neutralization of special elements (OS Command Injection) |
Critical | classes/PrestaShopAutoload.php:123 |
Fixed |
| BUG-003 | Improper control of filename for inclusion (PHP Remote File Inclusion) |
Critical | admin/filemanager/execute.php:51 |
Fixed |
| BUG-004 | Non-constant file inclusion (Local File Inclusion) |
High | admin/filemanager/dialog.php:154 |
Fixed |
| BUG-005 | Executing non-constant commands (Command Injection) |
Critical | modules/ps_themecusto/controllers/admin/AdminPsThemeCustoAdvanced.php:394 |
Fixed |
| BUG-006 | Potential security risk in Memcached authentication | Medium | provideServersSetting() |
Pending Review |
| BUG-007 | Weak hash usage (md5, sha1) for caching and file lookups | Medium | SmartyLazyCache.php, AssetRegistry.php, others |
Partially fixed |
| BUG-008 | $parent or $storeFolder traversal used in directory walking logic | Medium | filemanager/execute.php, create_folder.php, etc. |
Fixed |
| BUG-009 | No validation on file path before inclusion | High | Various: require_once $path . 'config.php' |
Fixed |
| BUG-010 | Missing validation in recursive config.php loader | High | filemanager/execute.php scripts, repeated pattern |
Fixed |
🔍 Analysis & Patterns
Common Themes
- Code Execution Risks: Multiple cases of
eval()usage leading to code injection vulnerabilities. - OS Command Injection: Use of
exec()andsystem()without proper sanitization. - Insecure File Inclusion: Dynamic file paths being required without strict validation.
- Weak Cryptography: Widespread use of md5() and sha1() for cache hashing and logic control
- UI Injection & Misuse: Unsafe usage of AJAX, folder creation without path sanitization.
- File Inclusion Risks: Multiple unvalidated require_once and recursive path walkers.
Root Causes Identified
- Unsafe use of
eval()andexec() - Lack of strict input validation in file inclusion paths
- Direct use of user-controlled variables in system commands
- Improper user authentication handling in Memcached configuration
- Legacy coding patterns lacking strict validation and type enforcement
- Lack of developer-side error handling for configuration path failures
Proposed Fixes
✅ Replaced eval() with class_alias() or safe alternatives.
✅ Replaced unsafe exec() calls with finfo_file() or fallback extension mapping.
✅ Restricted dynamic file inclusion using whitelisted languages and realpath containment.
✅ Rewrote folder walkers using realpath() + $base_path validation to prevent traversal.
✅ Hardened jQuery path manipulation logic and backend endpoints.
🔄 Still under review: Full refactoring of md5()/sha1() usage in caching (partial replacement complete).
📌 Next Steps
- Run additional security tests to validate the fixes.
- Implement automated scanning for command execution risks (
exec,system,passthru). - Enhance logging & error handling to detect unauthorized file inclusion attempts.
- Review Memcached authentication settings to ensure secure credentials handling.
- Implement pre-commit security hooks to detect dangerous patterns like eval, exec, require $_GET, etc.
- Think about the hash('sha256', etc) usage, is it fine or need to change?