Fix: Prevent object injection via unserialize on LogViewer encrypted URL params

[pandigresik]

Pull Request: fix: Prevent object injection via unserialize on LogViewer encrypted URL params

Description

Memperbaiki kerentanan CWE-502 (Deserialization of Untrusted Data) pada LogViewerController. Empat pemanggilan Crypt::decrypt() yang memproses parameter URL f, l, dl, clean, del dari request user memakai mode default unserialize = true, sehingga penyerang yang memiliki APP_KEY dapat mengirim serialized object berbahaya yang langsung di-unserialize(). Semua pemanggilan diganti ke Crypt::decryptString() (tanpa unserialize) karena nilai yang diharapkan adalah string path, sekaligus menambahkan test regresi.

Changes made:

1. [Security (Controller)]: app/Http/Controllers/LogViewerController.php — mengganti 4 pemanggilan Crypt::decrypt(...) pada input user menjadi Crypt::decryptString(...) (baris 84, 88, 146, dan 181 — pathFromInput untuk flow download/clean/delete).
2. [Security (View)]: resources/views/vendor/laravel-log-viewer/log.blade.php — mengganti 5 pemanggilan Crypt::encrypt(...) menjadi Crypt::encryptString(...) agar proses round-trip tetap konsisten dengan decryptString (menghindari double-serialization).
3. [Test (Regression)]: tests/Feature/LogViewerControllerTest.php — test baru yang mengirim payload serialized object gadget (seolah-olah penyerang memiliki APP_KEY) ke route setting.info-sistem dan memastikan __wakeup() tidak terpanggil (object injection tidak terjadi).

Reason for change:

• Object injection / RCE: Crypt::decrypt() secara default menjalankan unserialize() pada isi ciphertext. Karena payload berasal dari request user, penyerang yang memiliki APP_KEY bisa menyisipkan serialized object berbahaya → POP-chain → potensi RCE.
• Nullify attack surface: Dengan decryptString, nilai didekripsi sebagai string murni tanpa unserialize, sehingga seluruh attack vector object injection ditutup.
• Konsistensi enkripsi: View harus memakai encryptString agar nilai yang dikirim adalah string path mentah, bukan serialize(path).

Impact of change:

✅ Security: Menghilangkan unserialize() pada data tidak tepercaya — menutup CWE-502 / OWASP A08:2021.
✅ Attack surface reduction: Nilai terdekripsi kini hanya bisa berupa string path, memperkecil kemungkinan manipulasi tipe data.
✅ Regression guard: Test otomatis memastikan refactor tidak memperkenalkan kembali unserialize pada input user di masa depan.

Related Issue

• Fixes https://github.com/OpenSID/wiki-keamanan/issues/58

Steps to Reproduce

Before fix (problem):

1. Penyerang memiliki APP_KEY (mis. terbaca dari .env, repo, atau known-key).
2. Penyerang membuat payload Crypt::encryptString(serialize(new Gadget)).
3. Navigasi ke setting/info-sistem?f=<payload>.
4. Server memanggil Crypt::decrypt($f) → unserialize($f) → object terinstansiasi (__wakeup/__destruct terpanggil).
5. ❌ Object injection / potensi RCE berhasil dilakukan.

After fix (solution):

1. Server memanggil Crypt::decryptString($f) → nilai didekripsi sebagai string mentah tanpa unserialize.
2. Request berisi serialized object hanya menghasilkan string serialisasi, bukan instansiasi object.
3. ✅ __wakeup() gadget tidak pernah terpanggil — object injection tertutup.
4. ✅ Alur normal melihat/mengunduh/membersihkan/menghapus log tetap berfungsi.

Testing on related features:

• Lihat file log (navigasi ?l=...) ✅ Working
• Unduh file log (?dl=...) ✅ Working
• Bersihkan file log (?clean=...) ✅ Working
• Hapus file log (?del=...) ✅ Working
• Hapus semua file log (?delall=true) ✅ Working

Checklist

• Mengikuti aturan penulisan script (OpenSID)
• Mengikuti proses review pull request
• Membuat unit/integration test untuk memverifikasi fix
• Manual testing dilakukan di environment development
• Tidak ada console errors atau warnings
• Kode telah direview oleh minimal 1 orang

Technical Details

Technical Explanation

Akar masalah: Illuminate\Encryption\Encrypter::decrypt($payload, $unserialize = true) mengembalikan unserialize($decrypted) secara default. Semua panggilan di LogViewerController memakai default ini pada input user, sehingga isi ciphertext — yang bisa berisi serialized object buatan penyerang — di-unserialize.

// Sebelum (rentan):
$this->log_viewer->setFolder(Crypt::decrypt($this->request->input('f')));

// Sesudah (aman):
$this->log_viewer->setFolder(Crypt::decryptString($this->request->input('f')));

decryptString() memanggil decrypt($payload, false) sehingga mengembalikan string mentah tanpa unserialize. Karena nilai yang dipakai seharusnya adalah path (string), ini tidak mengubah perilaku fungsional log viewer.

Konsistensi view: Semula view memakai Crypt::encrypt($value) = encrypt(serialize($value)) (double-serialize). Diganti Crypt::encryptString($value) = encrypt($value, false) agar payload yang dikirim adalah string path mentah, yang konsisten dengan decryptString() di sisi server.

Test regresi (LogViewerControllerTest.php):

• Gadget class ObjectInjectionGadget memiliki __wakeup() yang menandai static::$wokenUp = true.
• Payload dikirim via Crypt::encryptString(serialize(new ObjectInjectionGadget)) — meniru penyerang yang memiliki APP_KEY.
• Dengan decryptString, __wakeup() tidak terpanggil → test PASS. Jika dikembalikan ke Crypt::decrypt (unserialize=true), test FAIL → terbukti menangkap regresi (red/green terverifikasi).

Configuration changes

Tidak ada perubahan konfigurasi. (Catatan: perubahan DB_PASSWORD di .env.testing hanya untuk environment testing lokal dan bukan bagian dari PR ini.)

Dependencies added

No new dependencies

Testing

Manual Testing

• Login sebagai admin → menu Setting → Info Sistem (/setting/info-sistem)
• Klik file log untuk melihat isi log (parameter l)
• Klik tombol Unduh (parameter dl)
• Klik tombol Bersihkan File (parameter clean)
• Klik tombol Hapus File (parameter del)
• Klik tombol Hapus Semua File (parameter delall)
• Regression Testing — pastikan semua aksi log viewer tetap berfungsi normal

Automated Testing

• Pest — tests/Feature/LogViewerControllerTest.php — "user-supplied log params are decrypted without unserialization"

Browser Compatibility (if applicable)

N/A — tidak ada perubahan visual UI

Screenshots / Video

Breaking Changes

Tidak ada perubahan perilaku fungsional. Satu catatan: URL bookmark/link lama yang dibuat dengan Crypt::encrypt() (double-serialized) tidak lagi ter-resolve dengan benar setelah decryptString(). Secara fungsional, link tersebut berasal dari halaman itu sendiri dan selalu di-render ulang dengan encryptString, sehingga normalnya tidak bermasalah.

Migration Guide

Not required

References

• Issue wiki-keamanan #58
• CWE-502: Deserialization of Untrusted Data
• OWASP A08:2021 – Software and Data Integrity Failures

---

Additional notes:

• Test dibuktikan red/green: dengan kode lama (Crypt::decrypt) test gagal (gadget __wakeup terpanggil), dengan kode baru (decryptString) test lulus.
• Nilai APP_KEY adalah secret yang tidak boleh bocor; fix ini mengurangi dampak bila key ter-expose (defense-in-depth), bukan pengganti untuk melindungi APP_KEY.

[github-actions]

🔄 AI PR Review sedang antri di server...

Proses review akan segera dimulai di background — hasil akan muncul sebagai komentar setelah selesai.
Powered by CrewAI · PR #1716