hoelee
e457e14b30
Security hardening: - bcrypt password auth via password_hash()/password_verify(), no default creds - Session CSRF token verified with hash_equals() - Secure session cookies: HttpOnly, SameSite=Strict, Secure over HTTPS - Nonce-based Content-Security-Policy, frame/clickjack + nosniff headers - realpath() root containment + symlink exclusion to block traversal - Server-side upload size + MIME(finfo) + extension validation - Configurable IP allow/deny with CIDR, optional HTTPS enforcement - Fixed 500ms login delay to slow brute-force guessing Features (single PHP file, no CDN): - New Bootstrap 5 responsive UI with light/dark theme toggle - Upload (drag+drop), download-only links, delete, rename, copy/move, mkdir - Folder ZIP download, in-browser text editor (<=2MiB) - Multi-user, per-user read-only + per-user root directories - All frontend assets vendored locally under assets/ (349K) Docs: full config reference, password hash guide, Tor hidden-service guide.
234 lines
7.7 KiB
Markdown
234 lines
7.7 KiB
Markdown
# Simple File Manager - Code Review & Improvement Recommendations
|
|
|
|
## Current State
|
|
- **Fork**: `hoelee/simple-file-manager` (based on jcampbell1/simple-file-manager)
|
|
- **Original Inspiration**: prasathmani/tinyfilemanager (v2.6, Nov 2024)
|
|
- **Current File**: Single `index.php` (~20KB, 494 lines)
|
|
- **Last Update**: 2040331 - "default sort ordering is directory first, then name"
|
|
|
|
---
|
|
|
|
## 🔴 CRITICAL Security Issues
|
|
|
|
### 1. **No Authentication by Default** (`$PASSWORD = ''`)
|
|
- **Risk**: Anyone with URL access gets full file system access
|
|
- **Fix**: Enforce password or disable by default with clear warning
|
|
- **Reference**: TinyFM v2.6 has `$use_auth = true` with bcrypt password hashing
|
|
|
|
### 2. **Weak Path Traversal Protection**
|
|
```php
|
|
// Current: Only checks prefix, can be bypassed
|
|
if(substr($tmp, 0,strlen($tmp_dir)) !== $tmp_dir) err(403,"Forbidden");
|
|
```
|
|
- **Issues**: Symlinks, case-insensitive FS, Unicode normalization bypasses
|
|
- **Fix**: Use `realpath()` on both paths AFTER resolving symlinks
|
|
|
|
### 3. **XSRF Token in Cookie (Not HttpOnly)**
|
|
```php
|
|
setcookie('_sfm_xsrf', bin2hex(openssl_random_pseudo_bytes(16)));
|
|
```
|
|
- **Risk**: Accessible via JavaScript (XSS → CSRF bypass)
|
|
- **Fix**: Add `HttpOnly; Secure; SameSite=Strict` flags
|
|
|
|
### 4. **Timing Attack on Password Check**
|
|
```php
|
|
sha1($t.$_POST['p']) === sha1($t.$PASSWORD) // String comparison leaks timing
|
|
```
|
|
- **Fix**: Use `hash_equals()` for constant-time comparison
|
|
|
|
### 5. **No File Type Validation on Upload (MIME Only)**
|
|
```php
|
|
foreach($disallowed_patterns as $pattern)
|
|
if(fnmatch($pattern, $_FILES['file_data']['name'])) err(403,...);
|
|
```
|
|
- **Risk**: Extension-based only, easily bypassed (e.g., `shell.php.jpg`, polyglots)
|
|
- **Fix**: Validate MIME type with `finfo_file()`, check magic bytes
|
|
|
|
### 6. **No Upload Size Limit Enforcement (Server-Side)**
|
|
- Only client-side JS check (`MAX_UPLOAD_SIZE`)
|
|
- **Fix**: Server-side check before `move_uploaded_file()`
|
|
|
|
### 7. **Information Disclosure in Errors**
|
|
```php
|
|
echo json_encode(['error' => ['code'=>intval($code), 'msg' => $msg]]);
|
|
```
|
|
- **Risk**: Stack traces, paths leaked in production
|
|
- **Fix**: Generic error messages in production mode
|
|
|
|
---
|
|
|
|
## 🟠 HIGH Priority Improvements
|
|
|
|
### 8. **Outdated jQuery (1.8.2 from 2012!)**
|
|
```html
|
|
<script src="//ajax.googleapis.com/ajax/libs/jquery/1.8.2/jquery.min.js"></script>
|
|
```
|
|
- **Risks**: XSS vulnerabilities, no modern browser support, deprecated APIs
|
|
- **Fix**: Update to jQuery 3.7+ or use vanilla JS (tinyfilemanager uses 3.6.1)
|
|
|
|
### 9. **No Content Security Policy (CSP)**
|
|
- Inline scripts/styles, external CDN → XSS risk
|
|
- **Fix**: Add CSP header, move inline JS to separate file or nonce
|
|
|
|
### 10. **No Rate Limiting / Brute Force Protection**
|
|
- Password auth has no lockout, delay, or logging
|
|
- **Fix**: Add exponential backoff, IP-based rate limiting
|
|
|
|
### 11. **Insecure Default Permissions**
|
|
```php
|
|
$allow_delete = true;
|
|
$allow_upload = true;
|
|
$allow_create_folder = true;
|
|
$allow_direct_link = true;
|
|
```
|
|
- **Risk**: Dangerous defaults for public deployment
|
|
- **Fix**: Secure defaults (all `false`), opt-in via config
|
|
|
|
### 12. **No HTTPS Enforcement**
|
|
- Password sent in cleartext over HTTP
|
|
- **Fix**: Check `$_SERVER['HTTPS']`, redirect or reject
|
|
|
|
---
|
|
|
|
## 🟡 MEDIUM Priority Improvements
|
|
|
|
### 13. **Missing Features from TinyFM v2.6**
|
|
| Feature | TinyFM | Current |
|
|
|---------|--------|---------|
|
|
| File editing (ace.js) | ✅ | ❌ |
|
|
| Syntax highlighting | ✅ | ❌ |
|
|
| Multi-user with permissions | ✅ | ❌ |
|
|
| Read-only users | ✅ | ❌ |
|
|
| User-specific directories | ✅ | ❌ |
|
|
| IP whitelist/blacklist | ✅ | ❌ |
|
|
| Chunked uploads (large files) | ✅ | ❌ |
|
|
| Online doc viewer (Google/MS) | ✅ | ❌ |
|
|
| Dark/Light theme | ✅ | ❌ |
|
|
| Multiple languages | ✅ | ❌ |
|
|
| Config file override | ✅ | ❌ |
|
|
| Hidden files toggle | ✅ | ❌ |
|
|
| File permissions/owner display | ✅ | ❌ |
|
|
| Search/filter | ✅ | ❌ |
|
|
| Copy/move/rename | ✅ | ❌ |
|
|
| Zip download | ✅ | ❌ |
|
|
| Image preview | ✅ | ❌ |
|
|
|
|
### 14. **Code Quality Issues**
|
|
- Mixed PHP/HTML (hard to maintain)
|
|
- No PSR standards, inconsistent formatting
|
|
- Global variables instead of dependency injection
|
|
- No autoloading, no Composer support
|
|
- Functions defined after usage (works but confusing)
|
|
|
|
### 15. **Accessibility Issues**
|
|
- No ARIA labels, no keyboard navigation for upload area
|
|
- Color-only indicators (drag-over state)
|
|
- No focus management
|
|
|
|
### 16. **Mobile/Responsive Gaps**
|
|
- Fixed width: `width:1024` (not responsive)
|
|
- Touch targets too small
|
|
- No viewport meta tag
|
|
|
|
---
|
|
|
|
## 🟢 LOW Priority / Nice to Have
|
|
|
|
### 17. **Modern PHP Practices**
|
|
- Type hints, strict types, scalar/return types
|
|
- PSR-12 coding standard
|
|
- Dependency injection container
|
|
- Unit tests (PHPUnit)
|
|
|
|
### 18. **Performance**
|
|
- No caching headers for static assets
|
|
- No gzip/compression hints
|
|
- Scandir() on every request (no caching)
|
|
|
|
### 19. **Developer Experience**
|
|
- No `.editorconfig`, no `phpstan`/`psalm` config
|
|
- No GitHub Actions CI
|
|
- No Dockerfile for testing
|
|
|
|
---
|
|
|
|
## 📋 Recommended Action Plan (Keeping Single File Constraint)
|
|
|
|
### Phase 1: Security Hardening (Week 1)
|
|
1. ✅ Enforce password auth by default
|
|
2. ✅ Fix path traversal with `realpath()`
|
|
3. ✅ Add `HttpOnly; Secure; SameSite=Strict` to cookies
|
|
4. ✅ Use `hash_equals()` for password check
|
|
5. ✅ Server-side upload size validation
|
|
6. ✅ MIME type validation with magic bytes
|
|
7. ✅ Generic error messages (configurable debug mode)
|
|
|
|
### Phase 2: Modernization (Week 2)
|
|
8. ✅ Update jQuery to 3.7.1 (or vanilla JS)
|
|
9. ✅ Add CSP header
|
|
10. ✅ Add rate limiting for auth
|
|
11. ✅ Secure defaults (disable upload/delete by default)
|
|
12. ✅ HTTPS enforcement check
|
|
|
|
### Phase 3: Feature Parity (Selective, Week 3)
|
|
13. ✅ Add file editing (ace.js via CDN)
|
|
14. ✅ Add syntax highlighting (highlight.js)
|
|
15. ✅ Add dark/light theme toggle
|
|
16. ✅ Add search/filter
|
|
17. ✅ Add copy/move/rename
|
|
18. ✅ Make responsive (viewport, flexbox)
|
|
|
|
### Phase 4: Polish (Week 4)
|
|
19. ✅ Accessibility improvements
|
|
20. ✅ Mobile UX
|
|
21. ✅ Config file support (optional external `config.php`)
|
|
22. ✅ PSR-12 formatting within single file
|
|
|
|
---
|
|
|
|
## 🔍 Reference: TinyFM v2.6 Key Differences
|
|
|
|
| Aspect | TinyFM v2.6 | Current Fork |
|
|
|--------|-------------|--------------|
|
|
| Lines of code | ~5,700 | ~494 |
|
|
| Auth system | Multi-user, bcrypt, readonly | Single password, sha1 |
|
|
| File operations | Full CRUD + zip + edit | List, upload, delete, mkdir |
|
|
| Security | IP rules, CSP, rate limit | Basic XSRF only |
|
|
| UI | Bootstrap 5, DataTables, responsive | Custom CSS, jQuery 1.8 |
|
|
| Extensibility | Config file, plugins | Hardcoded config only |
|
|
| PHP requirement | 7.4+ | 5.6+ (implied) |
|
|
|
|
---
|
|
|
|
## 💡 Decision Point
|
|
|
|
**Option A**: Harden current minimal fork (keep ~500 lines)
|
|
- Pros: Simple, fast, familiar
|
|
- Cons: Missing many useful features, security debt
|
|
|
|
**Option B**: Adopt TinyFM v2.6 as base, customize
|
|
- Pros: Battle-tested, feature-rich, active maintenance
|
|
- Cons: Larger (~230KB), more complex
|
|
|
|
**Option C**: Hybrid - cherry-pick security + key features from TinyFM
|
|
- Pros: Best of both worlds
|
|
- Cons: More integration work
|
|
|
|
**Recommendation**: **Option C** - The current fork is too minimal for production use. Cherry-pick:
|
|
1. All security fixes from TinyFM
|
|
2. Auth system (bcrypt, multi-user)
|
|
3. File editing + syntax highlighting
|
|
4. Responsive UI (Bootstrap 5)
|
|
5. Config file support
|
|
|
|
This keeps it ~single-file (with optional config.php) while gaining production-ready security and features.
|
|
|
|
---
|
|
|
|
## 📦 Files to Reference from TinyFM v2.6
|
|
- `tinyfilemanager.php` (main) - security patterns, auth, file ops
|
|
- Config structure - for optional `config.php` support
|
|
- CSS/JS CDN references - for modern deps
|
|
- Chunked upload logic - for large file support
|
|
|
|
*Generated: 2026-08-19* |