Jacek Pyziak
0776c4531e
Wygenerowano przez równoległą analizę czterech agentów: stack, architektura, konwencje, integracje, testy, baza danych oraz wykryte problemy i dług techniczny. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
98 lines
5.0 KiB
Markdown
98 lines
5.0 KiB
Markdown
# Concerns & Technical Debt — rank24.pl
|
|
|
|
## Security — CRITICAL
|
|
|
|
### Hardcoded Credentials (must fix before any public exposure)
|
|
|
|
| Secret | Location | Risk |
|
|
|--------|---------|------|
|
|
| MySQL password | `config.php` lines 2-5 | Full DB access if repo leaked |
|
|
| FTP password | `.vscode/ftp-kr.json`, `.vscode/sftp.json` | Full server access |
|
|
| DataForSEO API key | `autoload/class.Cron.php` ~lines 160, 262, 354 | API abuse / billing fraud |
|
|
| SMTP password | `autoload/class.S.php` ~lines 293-300 | Email spoofing |
|
|
|
|
**Remediation**: move all secrets to environment variables or a `.env` file excluded from VCS.
|
|
|
|
### SQL Injection
|
|
|
|
- `autoload/class.Cron.php` ~line 200: raw string concatenation in DELETE query
|
|
- `autoload/class.GoogleRank.php` lines 74, 96, 100, 136, 158, 162: raw string concat in UPDATE queries
|
|
- `autoload/class.DataBase.php` lines 15, 47, 82: mixed OPD with string building
|
|
|
|
**Remediation**: use Medoo's parameterized methods or PDO `bindValue()` for all dynamic values.
|
|
|
|
### Other Security Issues (MEDIUM)
|
|
|
|
- **Weak password hashing**: `md5($pass1)` in `autoload/class.DataBase.php` line 31 — use `password_hash()`
|
|
- **No CSRF protection**: state-changing AJAX operations in `ajax.php` lack CSRF tokens
|
|
- **Path traversal**: `autoload/class.DataBase.php` ~line 57 — user-supplied `image_folder` concatenated into file path without validation
|
|
- **Client-supplied MIME type**: file type validation in `class.DataBase.php` checks `$file['type']` (attacker-controlled)
|
|
- **Insecure deserialization**: `@unserialize()` used in `autoload/class.FileCache.php` line 43 and `autoload/opd.statement.php`
|
|
- **XSS**: `\S::get()` reads raw `$_POST`/`$_GET` without sanitization; values reach HTML output in multiple templates
|
|
|
|
## Technical Debt
|
|
|
|
### God Classes
|
|
|
|
- `autoload/class.S.php` — 700+ lines; handles sessions, email, DNS, CSV, URL, string utils, DB helpers. Should be split.
|
|
- `autoload/class.GoogleRank.php` — 300+ lines; proxy selection logic repeated 4+ times with no extraction.
|
|
- `autoload/class.Cron.php` — 400+ lines; hardcoded credentials, multiple large functions.
|
|
|
|
### Code Duplication
|
|
|
|
- Proxy selection + backoff UPDATE query repeated verbatim ~4 times in `class.GoogleRank.php`
|
|
- Google block-detection strings (`"Our systems have detected unusual traffic"`) duplicated in multiple methods
|
|
|
|
### Global State Anti-Pattern
|
|
|
|
Every class does `global $db, $mdb, $user, $config, $cache;` — no DI, no service container. Makes refactoring and testing very difficult.
|
|
|
|
### Two ORM Layers
|
|
|
|
Both `$db` (OPD) and `$mdb` (Medoo) are initialized and used. Older code paths use OPD raw queries; newer paths use Medoo. Inconsistent access patterns throughout.
|
|
|
|
### Deprecated PHP Patterns
|
|
|
|
- Old-style constructor: `function DataEdit()` in `autoload/class.DataEdit.php` line 32 (should be `__construct()`)
|
|
- `global` variable injection instead of constructor parameters
|
|
- Short open tags `<?` in templates — requires `short_open_tag = On` in php.ini
|
|
|
|
### Missing Abstractions
|
|
|
|
- No HTTP client wrapper — cURL used directly in `class.GoogleScraper.php`, no retry/logging
|
|
- No proxy manager class — proxy logic spread across `GoogleRank`, `GoogleSite`, `class.S`
|
|
- No logger — debugging done via `file_put_contents('google-rank.txt', ...)` and commented-out `\S::pre()`
|
|
- No centralized input validation layer — all `\S::get()` calls are point-of-use
|
|
|
|
## Reliability Concerns
|
|
|
|
### Cron Jobs
|
|
|
|
- No retry logic — single failure = job skipped until next run
|
|
- No max execution time enforcement — long foreach loops can time out silently
|
|
- `cron.php` outputs JSON directly — no structured logging for cron daemon
|
|
- API responses partially unchecked: `class.Cron.php` ~line 162 returns `'ok'` without verifying data integrity
|
|
|
|
### Scraping / Proxy
|
|
|
|
- Proxy rotation is deterministic (`ORDER BY used ASC LIMIT 1`) — predictable, easier for Google to detect
|
|
- No rate limiting or delays between requests in batch operations
|
|
- No exponential backoff — same fixed 10s timeout regardless of failure history (`CURLOPT_TIMEOUT = 10`)
|
|
- Block detection only covers known string patterns — new CAPTCHA formats would be missed
|
|
- `file_put_contents('google-rank.txt', $result)` in `class.GoogleRank.php` line ~185 — debug log left in production
|
|
|
|
### Error Handling
|
|
|
|
- Global `error_reporting` suppresses notices, warnings, deprecations — real errors can be masked
|
|
- cURL operations in `class.GoogleScraper.php` have no try/catch — silent failures return `-1`
|
|
- File operations in `class.DataBase.php` use `@` suppression — orphaned temp files possible
|
|
- No circuit breaker — scraping continues even after repeated proxy failures
|
|
|
|
## Minor Issues
|
|
|
|
- Debug variable `$debbbb` left in `autoload/class.S.php` line ~357
|
|
- Commented-out dead code in `autoload/class.Cron.php` lines 56-61
|
|
- Duplicate key in array: `autoload/class.Cron.php` lines ~364, ~376 — `'se_id'` set twice with different values
|
|
- Session-based state won't scale to multi-server deployment (no session locking)
|
|
- All proxy requests use `http://` (line ~120 `class.GoogleRank.php`) — credentials sniffable on network
|