fix: Remaining audit findings — OOM, security, error handling (#81) #85

Merged

jmiller merged 1 commits from fix/audit-remaining into main 2026-06-21 23:17:57 +00:00

Conversation 0 Commits 1 Files Changed 5

+185 -24

jmiller commented 2026-06-21 23:17:31 +00:00

Owner

Copy Link

Copy Source

Summary

Fixes all remaining critical issues and several medium findings from the codebase audit (#81).

Closes #73, closes #74, closes #75. Ref #81.

Fixes

#	Severity	Fix
#73	CRITICAL	S3Uploader streams file via CURLOPT_PUT/CURLOPT_INFILE — no more file_get_contents OOM
#74	CRITICAL	DatabaseDumper::dumpToFile() streams SQL to disk; BackupEngine uses addFile() not addFromString()
#75	HIGH	AkeebaImporter removes unserialize() — JSON only, skips legacy data
—	MEDIUM	$archiveName initialized before try (prevents undefined in catch)
—	MEDIUM	Plaintext archive deleted when AES encryption fails
—	MEDIUM	Temp SQL file cleaned up on both success and failure paths
—	MEDIUM	createArchiver() throws on unknown format instead of silent ZIP fallback
—	LOW	TarGzArchiver::close() uses try/finally to clean up intermediate .tar

Test Plan

• Full backup completes — database dumped via temp file, archive contains database.sql
• S3 upload works with streaming (no OOM on large files)
• Akeeba import with JSON filters works; serialized filters gracefully skipped
• Backup with invalid archive_format profile value shows clear error
• Backup with AES encryption failure: plaintext archive is removed
• tar.gz backup: no leftover .tar file after completion or failure

## Summary Fixes all remaining critical issues and several medium findings from the codebase audit (#81). Closes #73, closes #74, closes #75. Ref #81. ## Fixes | # | Severity | Fix | |---|----------|-----| | #73 | CRITICAL | S3Uploader streams file via `CURLOPT_PUT`/`CURLOPT_INFILE` — no more `file_get_contents` OOM | | #74 | CRITICAL | `DatabaseDumper::dumpToFile()` streams SQL to disk; `BackupEngine` uses `addFile()` not `addFromString()` | | #75 | HIGH | `AkeebaImporter` removes `unserialize()` — JSON only, skips legacy data | | — | MEDIUM | `$archiveName` initialized before try (prevents undefined in catch) | | — | MEDIUM | Plaintext archive deleted when AES encryption fails | | — | MEDIUM | Temp SQL file cleaned up on both success and failure paths | | — | MEDIUM | `createArchiver()` throws on unknown format instead of silent ZIP fallback | | — | LOW | `TarGzArchiver::close()` uses try/finally to clean up intermediate `.tar` | ## Test Plan - [ ] Full backup completes — database dumped via temp file, archive contains `database.sql` - [ ] S3 upload works with streaming (no OOM on large files) - [ ] Akeeba import with JSON filters works; serialized filters gracefully skipped - [ ] Backup with invalid `archive_format` profile value shows clear error - [ ] Backup with AES encryption failure: plaintext archive is removed - [ ] tar.gz backup: no leftover `.tar` file after completion or failure

jmiller added the component: enginecomponent: remote labels 2026-06-21 23:17:31 +00:00

jmiller added 1 commit 2026-06-21 23:17:31 +00:00

fix: remaining audit findings — OOM, security, error handling (#81) ... 9990240d2d

CRITICAL:
- #73: S3Uploader now streams file via CURLOPT_PUT/INFILE instead of
  loading entire file into RAM with file_get_contents
- #74: DatabaseDumper gains dumpToFile() that streams SQL to disk;
  BackupEngine uses addFile() instead of addFromString() to avoid
  holding the entire dump in memory
- #75: AkeebaImporter removes unserialize() — only uses json_decode,
  skips legacy serialized filter data to prevent object injection

MEDIUM (also fixed):
- BackupEngine: $archiveName initialized before try block (prevents
  undefined variable in catch)
- BackupEngine: plaintext archive deleted on encryption failure
- BackupEngine: temp SQL file cleaned up in both success and failure
- BackupEngine: createArchiver() throws on unknown format instead of
  silently falling back to ZIP
- TarGzArchiver: intermediate .tar cleaned up in finally block

Closes #73, closes #74, closes #75
Ref #81

jmiller merged commit a26343a76e into main 2026-06-21 23:17:57 +00:00

jmiller deleted branch fix/audit-remaining 2026-06-21 23:17:59 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

component: admin

Admin UI and views

component: api

REST API

component: engine

Backup/restore engine

component: remote

Remote storage (FTP/GDrive)

component: scheduler

Scheduled tasks

No labels component: engine component: remote

Priority -

Type -

Milestone

No items

No Milestone

Assignees

Clear assignees

jmiller (Jonathan Miller)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MokoConsulting/MokoSuiteBackup#85