128 lines
4.8 KiB
Markdown
128 lines
4.8 KiB
Markdown
# Concerns & Technical Debt
|
|
|
|
> Last updated: 2026-03-12
|
|
|
|
## Security Issues
|
|
|
|
### HIGH — Sensitive data logged to public file
|
|
**File**: `autoload/front/Controllers/ShopOrderController.php:32`
|
|
```php
|
|
file_put_contents('tpay.txt', print_r($_POST, true) . print_r($_GET, true), FILE_APPEND);
|
|
```
|
|
- Logs entire POST/GET (including payment data) to `tpay.txt` likely in webroot
|
|
- Possible information disclosure
|
|
- **Fix**: Remove log or write to non-public path (e.g., `/logs/`)
|
|
|
|
### HIGH — Hardcoded payment seed
|
|
**File**: `autoload/front/Controllers/ShopOrderController.php:105`
|
|
```php
|
|
hash("sha256", "ProjectPro1916;" . round($summary_tmp, 2) ...)
|
|
```
|
|
- Hardcoded secret in source — should be in `config.php`
|
|
|
|
### MEDIUM — SQL table name interpolated
|
|
**File**: `autoload/Domain/Integrations/IntegrationsRepository.php:31`
|
|
```php
|
|
$stmt = $this->db->query("SELECT * FROM $table");
|
|
```
|
|
- Technically mitigated by whitelist in `settingsTable()`, but violates "no SQL string concatenation" rule
|
|
- **Fix**: Use Medoo's native `select()` method
|
|
|
|
### MEDIUM — Path traversal in unlink()
|
|
**Files**: `autoload/Domain/Product/ProductRepository.php:1605,1617,2129,2163` and `autoload/Domain/Article/ArticleRepository.php:321,340,823,840`
|
|
```php
|
|
if (file_exists('../' . $row['src'])) {
|
|
unlink('../' . $row['src']);
|
|
}
|
|
```
|
|
- Path from DB, no traversal check
|
|
- A DB compromise could delete arbitrary files
|
|
- **Fix**:
|
|
```php
|
|
$basePath = realpath('../upload/');
|
|
$fullPath = realpath('../' . $row['src']);
|
|
if ($fullPath && strpos($fullPath, $basePath) === 0) {
|
|
unlink($fullPath);
|
|
}
|
|
```
|
|
|
|
### MEDIUM — Unsanitized output in templates
|
|
**Files**:
|
|
- `templates/articles/article-full.php` — article title and `$_SERVER['SERVER_NAME']` concatenated without escaping
|
|
- `templates/articles/article-entry.php` — `$url` and article titles not escaped
|
|
|
|
### MEDIUM — Missing CSRF tokens
|
|
- No evidence of CSRF tokens on admin panel forms
|
|
- State-changing POST endpoints (create/update/delete) are potentially CSRF-vulnerable
|
|
|
|
---
|
|
|
|
## Architecture Issues
|
|
|
|
### IntegrationsRepository too large (875 lines)
|
|
**File**: `autoload/Domain/Integrations/IntegrationsRepository.php`
|
|
Does too many things: settings CRUD, logging, Apilo OAuth, product sync, webhook handling, ShopPRO import.
|
|
**Suggested split**: `ApiloAuthManager`, `ApiloProductSyncService`, `ApiloWebhookHandler`, `IntegrationLogRepository`, `IntegrationSettingsRepository`
|
|
|
|
### ProductRepository too large (3583 lines)
|
|
**File**: `autoload/Domain/Product/ProductRepository.php`
|
|
Candidate for extraction of: pricing logic, image handling, cache management, Google feed generation.
|
|
|
|
### ShopProductController too large (1199 lines)
|
|
**File**: `autoload/admin/Controllers/ShopProductController.php`
|
|
|
|
### Helpers.php too large (1101 lines)
|
|
**File**: `autoload/Shared/Helpers/Helpers.php`
|
|
Static utility god class. Extract into focused service classes.
|
|
|
|
### Duplicate email logic
|
|
- `\Shared\Helpers\Helpers::send_email()` and `\Shared\Email\Email::send()` both wrap PHPMailer
|
|
- Should be unified in `\Shared\Email\Email`
|
|
- Documented in `docs/MEMORY.md`
|
|
|
|
### 47 `global $mdb` usages remain
|
|
- DI is complete in Controllers, but some Helpers methods still use `global $mdb`
|
|
- Should be gradually eliminated
|
|
|
|
---
|
|
|
|
## Dead Code / Unused Files
|
|
|
|
| File | Issue |
|
|
|------|-------|
|
|
| `libraries/rb.php` | RedBeanPHP — no references found in autoload, candidate for removal |
|
|
| `cron-turstmate.php` (note: typo) | Legacy/questionable cron handler |
|
|
| `devel.html` | Development artifact in project root |
|
|
| `output.txt` | Artifact file |
|
|
| `libraries/filemanager-9.14.1/` + `9.14.2/` | Duplicate versions |
|
|
|
|
---
|
|
|
|
## Missing Error Handling
|
|
|
|
- `IntegrationsRepository.php:163-165` — DB operations after Apilo token refresh lack try-catch
|
|
- `ShopOrderController.php:32` — `file_put_contents()` return value not checked
|
|
- `ProductRepository.php:1605` — `unlink()` without error handling
|
|
- `cron.php:2` — `error_reporting(E_ALL ^ E_NOTICE ^ E_STRICT ^ E_WARNING ^ E_DEPRECATED)` silences all warnings, hiding potential bugs
|
|
|
|
---
|
|
|
|
## Known Issues (from docs/TODO.md & docs/MEMORY.md)
|
|
|
|
| Issue | Location | Status |
|
|
|-------|----------|--------|
|
|
| Newsletter save/unsubscribe needs testing | `Domain/Newsletter/` | Open |
|
|
| Duplicate email sending logic | `Helpers.php` vs `Email.php` | Open |
|
|
| `$mdb->delete()` 2-arg pitfall | Documented in MEMORY.md | Known pitfall |
|
|
|
|
---
|
|
|
|
## Summary by Priority
|
|
|
|
| Priority | Count | Key Action |
|
|
|----------|-------|-----------|
|
|
| **Immediate** (security) | 5 | Remove tpay.txt logging, fix path traversal, move hardcoded secret to config |
|
|
| **High** (architecture) | 3 | Split IntegrationsRepository, unify email logic, add CSRF |
|
|
| **Medium** (quality) | 4 | Escape template output, add try-catch, remove dead files |
|
|
| **Low** (maintenance) | 3 | Remove rb.php, reduce Helpers.php, document helpers usage |
|