Project-Pro/drmaterac.pl
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
main
drmaterac.pl/.paul/codebase/concerns.md
Jacek Pyziak 80a3cb3814 update
2026-05-10 21:32:38 +02:00

8.7 KiB
Raw Permalink Blame History

Codebase Concerns

Analysis Date: 2026-05-10

Severity legend: CRITICAL (immediate fix), HIGH (next sprint), MEDIUM (planned), LOW (track).

Critical Security

Hardcoded credentials committed to repository — CRITICAL

  • Files: buy-by-phone.php (lines ~11–28: reCAPTCHA secret 6LeJAUUsAAAAAIyCrwFMrsw9yQLPgnCWCOHPcjc8, SMTP passwords 8njIZnAb, kM4BU_F_), app/config/parameters.php (DB password eRbZ]Ioh-0-2]fM+, app secret LStbzv3bfsMaq5dvsVR5wXt8dEpe63IAx2uOHffpbK9gy9x4EZsjWeRM, cookie keys)
  • Why: convenience during development — never moved to env vars
  • Impact: full DB compromise, mail account takeover, session forgery if repo or backups leak; secrets remain valid in git history
  • Fix: rotate ALL exposed credentials immediately → move to environment variables (or Docker secrets) → load via getenv() → purge from git history (git filter-repo / BFG) → add the file to .gitignore

Diagnostic / phpinfo files exposed in webroot — CRITICAL

  • Files: diag_20792_tmp.php (2.6 KB), diag_upload_tmp.php (4.4 KB), diag_fix_tmp.php (14 B), info.php (14 B — almost certainly <?php phpinfo();)
  • Why: ad-hoc debug scripts left after a 2026-03-05 incident (feat: update diagnostic scripts and configuration for improved product data handling, commit 194405bc)
  • Impact: info.php discloses full PHP environment to anyone; diag scripts run raw SQL with weak $_GET['key'] !== 'diag2026' gate (security through obscurity, easily bypassed)
  • Fix: delete these 4 files now; if production deploy mirrors repo, also delete on server; consider reverting commit 194405bc

Unsanitized $_POST / $_GET in custom scripts — HIGH

  • Files: buy-by-phone.php lines 29–58 (17 direct $_POST[...] accesses) and lines 83–94 (POST values injected into HTML email body — XSS via mail / SMTP header injection); category-description.php line 7 ($url = $_GET['url'])
  • Why: written outside PrestaShop conventions
  • Impact: header injection, mail-relay abuse, stored XSS in support inboxes
  • Fix: replace with Tools::getValue('field', '') (PrestaShop sanitizer) and escape on output (htmlspecialchars, Tools::safeOutput); validate email with Validate::isEmail(); add reCAPTCHA verification result check before send

SQL string concatenation in custom scripts — HIGH

  • Files: diag_20792_tmp.php lines 23–49, diag_upload_tmp.php lines 22–50, import-product.php lines 63–93
  • Why: legacy PrestaShop pattern (pSQL() instead of bound parameters)
  • Impact: any unsanitized input flowing into these queries is an injection vector
  • Fix: delete diag scripts; refactor import-product.php to use Db::getInstance() with proper escaping or Doctrine DBAL parameterized queries; never concatenate user input into SQL

Database backups + 5 MB error log inside webroot — HIGH

  • Files: iadmin/backups/ (e.g. 1698930967-7683b7af.sql.bz2), iadmin/errors.log (5 MB, 26 months of PHP errors), errors.log (root)
  • Why: standard PrestaShop default location; iadmin/ is the renamed admin folder (still under DocumentRoot)
  • Impact: SQL dumps directly downloadable if URL guessed; error log discloses paths and module internals
  • Fix: move backups outside DocumentRoot or Deny from all via .htaccess; rotate logs to /var/log/...; add errors.log, *.log, iadmin/backups/, iadmin/errors.log to .gitignore

Tech Debt

Backup and obsolete files committed — HIGH

  • Files: backup_before_patch/As4SearchEngine.php.backup (160 KB), backup_before_patch/As4SearchEngineDb.php.backup (3.4 KB), backup_before_patch/pm_advancedsearch4.php.backup (279 KB)
  • Why: manual snapshot taken before patching the search engine module
  • Impact: bloats repo, confuses code search, may shadow current implementation in IDEs
  • Fix: delete the directory; rely on git log for history; add *.backup, backup_before_patch/ to .gitignore

Oversized root-level scripts — MEDIUM

  • Files: import-product.php (813 lines / 32 KB — fetches and applies external XML feed), buy-by-phone.php (122 lines — manual PHPMailer + reCAPTCHA)
  • Why: written as single-file utilities
  • Impact: hard to test, no transaction safety in importer, no logging of import results
  • Fix approach: extract import-product.php into a proper PrestaShop module or CLI command (bin/console-style), wrap DB writes in transactions, add structured logging; convert buy-by-phone.php into a ModuleFrontController

429-line .htaccess with hardcoded SEO redirects — MEDIUM

  • File: .htaccess (~100 manual RewriteRule ^...$ /s/.../...? redirects, lines 14–370+)
  • Impact: not maintainable, regex conflicts likely, no audit trail
  • Fix: migrate to a database-driven redirect table + module that emits 301s in hookActionDispatcher (or use modules/x13linkrewrite/)

107 undocumented core overrides — MEDIUM

  • Files: override/classes/ (31 files — Cart.php, CartRule.php, Product.php, Hook.php, Dispatcher.php, controller/FrontController.php, controller/ProductListingFrontController.php, …) plus 76 more under override/
  • Why: each accumulated over time to extend core behavior
  • Impact: every PrestaShop upgrade requires manual override review; no inline doc states why each override exists
  • Fix: add a header comment to each override stating purpose + ticket; consider moving simple cases to module hooks

Stale stack — PrestaShop 1.7.8.11 / PHP 7.4 — MEDIUM

  • File: app/AppKernel.php line 35 (const VERSION = '1.7.8.11')
  • Why: production stability
  • Impact: missing security patch 1.7.8.12; PHP 7.4 reached EOL 2022-11-28 (no upstream security fixes); core 1.7.x EOL since 2021-10
  • Fix: apply 1.7.8.12 patch first (minor), then plan migration to PrestaShop 8.x or 9.x; PHP 8.2 already in runtime — verify all modules under PHP 8.x compat-checker

iadmin/autoupgrade/ contains prestashop_1.7.5.1.zip left in webroot — MEDIUM

  • Why: artifact of a past upgrade attempt
  • Impact: downloadable archive exposing core source paths
  • Fix: delete

Performance

Custom cross-sell module clean — no concerns

  • File: modules/crosssellpro/crosssellpro.php
  • Uses Product::getAccessoriesLight() (efficient) + Product::getProductsProperties(); filters with Shop::addSqlAssociation; respects DEFAULT_LIMIT = 12
  • No N+1 detected

No application-level caching active

  • Memcached configured but disabled (app/config/parameters.php)
  • Page cache module installed (modules/pagecache/) — verify it is enabled in production
  • Fix: enable Memcached + verify pagecache configuration; add OPcache validation if not on

Fragile Areas

Override chain in override/classes/

  • Why fragile: Cart::getOrderTotal() extension depends on ets_promotion module presence (override/classes/Cart.php line ~55); Hook::exec extension depends on pagecache (override/classes/Hook.php line ~42)
  • Common failures: removing one of those modules without cleaning up the override leaves dead branches
  • Safe modification: read the override + the referenced module's hook surface together; never edit core classes/ directly

Recurring header-modification errors in admin

  • Source: iadmin/errors.log shows repeated errors at AdminCartRulesController.php:121 (output before header() call)
  • Trigger: editing cart rules in back-office
  • Likely cause: a module emitting whitespace before <?php or echoing in init()

MySQL "server has gone away" entries

  • Source: iadmin/errors.log
  • Likely cause: long-running scripts (import-product.php?) exceeding wait_timeout
  • Fix: increase wait_timeout/mysql.connect_timeout in PHP config or call Db::getInstance()->disconnect() + reconnect for long-running batch jobs

Test Coverage Gaps

  • Custom modules have NO tests — modules/crosssellpro/, modules/caraty/
  • No tests for import-product.php (highest-risk script — bulk DB writes, no rollback)
  • No tests for buy-by-phone.php (handles user input + sends mail)
  • Risk: regressions land silently; manual browser testing is the only safety net

Documentation Gaps

  • No README.md at repo root (only PrestaShop's stock files)
  • 31 core overrides — none have purpose comments
  • iadmin/errors.log retains 26 months of error history but no rotation policy

Deletion Candidates (Quick Wins)

diag_20792_tmp.php
diag_fix_tmp.php
diag_upload_tmp.php
info.php
errors.log
backup_before_patch/
iadmin/errors.log
iadmin/autoupgrade/prestashop_1.7.5.1.zip
themes/classic.zip   # (verify needed)
themes/leo_gstore.zip

Add to .gitignore: errors.log, *.log, *.backup, backup_before_patch/, iadmin/backups/, iadmin/errors.log, iadmin/autoupgrade/*.zip.

Concerns audit: 2026-05-10 Update as issues are fixed or new ones discovered

Reference in New Issue View Git Blame Copy Permalink