Fix: validasi valid_file menolak PDF sah karena string generik "function"

[pandigresik]

Description

Rule custom valid_file membaca seluruh isi file dengan File::get($value) lalu menjalankan regex teks /<\?php|<script|function|__halt_compiler|<html/i tanpa membedakan jenis format. Karena PDF/Office adalah file biner yang dapat memuat string function secara normal pada struktur dan metadata-nya, dokumen yang sah ditolak dengan pesan "Format Jenis berkas yang anda unggah berbahaya."

Perbaikan ini memindahkan logika ke App\Rules\SafeFileContent dan membelah pemeriksaan menjadi dua lap: penanda skrip universal (<?php, <?=, <script, __halt_compiler) untuk semua format, dan pola XSS tambahan yang hanya dijalankan pada format berbasis markup (SVG/XML/HTML). Token generik function dihapus, sementara proteksi terhadap PDF yang disisipi kode PHP justru dipertahankan dan diperkuat.

Changes made:

1. Refactor: Logika valid_file dipindahkan dari closure di app/Providers/AppServiceProvider.php ke class baru app/Rules/SafeFileContent.php agar dapat diuji terpisah (TDD) dan mengikuti pola App\Rules\SecureFileUpload yang sudah ada.
2. Fix: Token generik function dan <html dihapus dari pola global; digantikan dua konstanta — SCRIPT_SIGNATURES (semua format) dan MARKUP_DANGERS (hanya SVG/XML/HTML, mencakup on*= event handler, javascript:, vbscript:, <foreignObject>, <iframe>, <embed>, <object>, data:text/html).
3. Fix: File::get($value) diganti fread() dengan batas 1 MB (SCAN_LIMIT) — pemindaian tidak lagi memuat seluruh file ke memori.
4. Fix: Guard kondisi ! $value instanceof UploadedFile || ! $value->isValid() ditambahkan agar input non-file ditolak secara deterministik.
5. Hardening: Header X-Content-Type-Options: nosniff dipasang di app/Http/Middleware/SecurityHeaders.php pada semua environment (sebelumnya middleware hanya memasang CSP di production). properti unwantedHeaders yang dideklarasikan tetapi tidak pernah dipakai kini benar-benar diterapkan.
6. Hardening: location blocker di .docker/nginx/conf.d/default.conf untuk menolak eksekusi skrip di bawah /storage/ — path ini disajikan langsung oleh webserver melalui symlink public/storage sehingga melewati seluruh middleware Laravel.
7. Test: 3 file test baru — tests/Unit/ValidFileContentTest.php (15 test), tests/Unit/SecurityHeadersTest.php (3 test), tests/Arch/NginxUploadGuardTest.php (3 test).

Reason for change:

• False positive pada dokumen sah: file RENJA PERUBAHAN KECAMATAN PECANGAAN 2025_compressed.pdf (702.935 byte, application/pdf, isValid() = true, di bawah max:2048) ditolak solely karena mengandung 14 kemunculan string Function. Regex lama tidak memberikan nilai keamanan apa pun untuk file biner — PHP di dalam PDF tidak pernah dieksekusi karena file disajikan sebagai application/pdf dengan Content-Disposition: attachment dan ekstensi dibuat server-side.
• Proteksi terhadap penyisipan skrip tetap dijaga: file dengan header %PDF- yang disisipi <?php, dan file .php yang menyamar sebagai PDF, keduanya kini ditolak oleh SCRIPT_SIGNATURES.
• Memperkuat vektor XSS yang sebenarnya: file hasil upload disajikan dari symlink public/storage → storage/app/public dan dilayani nginx secara langsung, sehingga melewati middleware. SVG yang diizinkan pada DokumenRequest menjadi same-origin URL yang dapat dieksekusi browser. Regex versi issue hanya menangkap <script; versi ini juga menangkap onload=, xlink:href="javascript:", dan <foreignObject>.
• Lap keamanan berlapis di webserver: location ~ \.php$ pada default.conf memiliki try_files yang dikomentari sehingga tidak ada verifikasi keberadaan file; guard /storage/ menutup celah ini dan ditulis sebelum location ~ \.php$ agar tidak ditimpa oleh regex location yang di Evaluasi berurutan.

Impact of change:

✅ Dokumen binary sah: PDF/DOC/DOCX/XLS/XLSX/PPT/PPTX/JPG/PNG/GIF tidak lagi ditolak akibat string yang kebetulan cocok.
✅ Deteksi penyisipan skrip: <?php, <?=, <script, __halt_compiler tetap terdeteksi pada semua format termasuk di dalam PDF.
✅ Proteksi SVG/HTML: meningkat dari 3 penanda menjadi 9 penanda (event handler, javascript:, foreignObject, iframe, embed, object, data:text/html).
✅ Performa: Pemindaian dibatasi 1 MB dan memakai stream fread, bukan File::get() yang memuat seluruh file.
✅ Keamanan header: X-Content-Type-Options: nosniff mencegah content-type sniffing pada seluruh response web & API.
✅ Keterbacaan: Logika validasi terisolasi di class khusus yang mengikuti ValidationRule Laravel, bukan closure inline.

Related Issue

• Solution for fix related to issue #1761

Closes #1761

Steps to Reproduce

Before fix (problem):

1. Masuk sebagai admin, buka menu Informasi → Form Dokumen.
2. Klik tambah dokumen, isi nama_dokumen, jenis_dokumen, retention_days, description, status.
3. Unggah file RENJA PERUBAHAN KECAMATAN PECANGAAN 2025_compressed.pdf (702.935 byte, MIME application/pdf).
4. Verifikasi bahwa file memenuhi semua syarat: isValid() = true, upload error 0, ukuran 687 KB di bawah max:2048.
5. Periksa isi file: grep -aoiE '<\?php|<script|function|__halt_compiler|<html' *.pdf | sort | uniq -c menghasilkan 14 Function.
6. ❌ Validasi valid_file mengembalikan false → pesan "Format Jenis berkas yang anda unggah berbahaya." despite file valid.

After fix (solution):

1. Login sebagai admin, buka Informasi → Form Dokumen.
2. Tambahkan dokumen baru dengan file PDF yang sama.
3. Isi field wajib lainnya.
4. Klik simpan.
5. ✅ Berkas diterima, dokumen tersimpan, dan file dapat diunduh normal dari menu download.

Testing on related features:

• Form Dokumen — upload PDF berisi string function ✅ Lolos
• Form Dokumen — upload SVG berisi <script> ✅ Ditolak
• Artikel / Album / Galeri / Slide / Profil — upload gambar ✅ Lolos (regresi)
• Prosedur / Regulasi — upload PDF ✅ Lolos (regresi)
• Komplain (frontend & API) — upload lampiran ✅ Lolos (regresi)
• Unggah PHP menyamar .pdf ✅ Ditolak

Checklist

• I have complied with script writing rules
• I have followed pull request review process
• I have created unit test to verify the fix
• Manual testing has been done in development environment
• No console errors or warnings
• Code has been reviewed by at least 1 person

Technical Details

Technical Explanation

Diagram alur validasi valid_file setelah perubahan:

UploadedFile
     │
     ├─ bukan UploadedFile / !isValid() ──────────────► REJECT
     │
     ▼
readHead() → fread 1 MB (bukan File::get() seluruh file)
     │
     ├─ cocok SCRIPT_SIGNATURES ? ────────────────────► REJECT
     │  /<?php|<??=|<script\b|__halt_compiler/i
     │  (berlaku untuk SEMUA format, termasuk PDF)
     ▼
getMimeType() ∈ MARKUP_MIMES ? (svg, html, xhtml, xml)
     │
     ├─ ya & cocok MARKUP_DANGERS ? ─────────────────► REJECT
     │  on*=, javascript:, <foreignObject>, <iframe>, ...
     ▼
   ACCEPT

Kunci perbedaannya: pemeriksaan konten yang spesifik per-format hanya dijalankan bila formatnya memang bisa dieksekusi browser. PDF/Office tidak masuk jalur kedua, sehingga string biner yang wajar tidak lagi berpengaruh, sementara marker skrip universal tetap diperiksa untuk semua format.

Perubahan di app/Providers/AppServiceProvider.php:

-    $contains = preg_match('/<\?php|<script|function|__halt_compiler|<html/i', File::get($value));
-    if ($contains) {
-        return false;
-    }
-
-    return true;
+    $failed = false;
+
+    (new SafeFileContent())->validate($attributes, $value, function () use (&$failed) {
+        $failed = true;
+    });
+
+    return ! $failed;

Perubahan di .docker/nginx/conf.d/default.conf:

+    location ~* ^/storage/.*\.(php|php[0-9]?|phtml|phar|pl|py|rb|sh|bash|cgi|jsp|asp|aspx|htaccess)$ {
+        deny all;
+    }
+
     error_page 404 /index.php;

Blok ini harus berada sebelum location ~ \.php$. Nginx mengevaluasi seluruh regex location sesuai urutan konfigurasi dan berhenti pada match pertama.

Perubahan di app/Http/Middleware/SecurityHeaders.php:

+        foreach ($this->unwantedHeaders as $header) {
+            $response->headers->remove($header);
+        }
+
+        $response->headers->set('X-Content-Type-Options', 'nosniff');

Configuration changes

Tidak ada perubahan pada file konfigurasi aplikasi (.env, config/*.php). Perubahan pada .docker/nginx/conf.d/default.conf memerlukan restart/reload container nginx (docker compose restart nginx) agar guard /storage/ aktif.

Dependencies added

No new dependencies.

Testing

Manual Testing

• Upload PDF valid yang mengandung string function melalui Form Dokumen → berhasil
• Upload PDF valid tanpa konten bermasalah → berhasil
• Upload SVG berisi <script>alert(1)</script> → ditolak
• Upload file .pdf yang isinya pure PHP → ditolak
• Upload file .pdf ber-header %PDF- tapi disisipi <?php → ditolak
• Regression Testing — Artikel, Album, Galeri, Slide, Profil, Prosedur, Regulasi, Komplain → tidak ada regresi

Automated Testing

• Unit Test — tests/Unit/ValidFileContentTest.php (15 test, 18 assertion)
  • pdf valid yang mengandung string Function tetap lolos
  • pdf yang menyamar dengan header pdf tetapi berisi kode php ditolak
  • file php yang diberi ekstensi pdf ditolak
  • svg dengan tag script / event handler onload / javascript pada xlink href / foreignObject ditolak
  • svg bersih lolos
  • gambar png dan jpeg asli tetap lolos
  • file yang menyamar sebagai gambar tetapi berisi html ditolak
• Unit Test — tests/Unit/SecurityHeadersTest.php (3 test, 3 assertion)
  • menambahkan X-Content-Type-Options nosniff di production
  • nosniff tetap dipasang di luar production
  • tidak membocorkan versi server
• Arch Test — tests/Arch/NginxUploadGuardTest.php (3 test, 11 assertion)
  • memblokir eksekusi skrip di bawah /storage/
  • guard /storage/ ditulis sebelum location .php agar tidak ditimpa
  • blokir mencakup daftar ekstensi eksekutable yang umum
• php vendor/bin/pest tests/Unit tests/Arch → 501 passed (1193 assertions)
• php vendor/bin/pest tests/Feature/Security tests/Feature/Upload tests/Feature/Surat tests/Feature/Validation tests/Feature/Content → 117 passed, 4 failure pre-existing (terverifikasi identik pada rilis-dev via git stash)
• vendor/bin/pint --test pada 6 file yang diubah → passed
• nginx -t pada .docker/nginx/conf.d/default.conf → syntax is ok

Browser Compatibility

Tidak ada perubahan UI/frontend. Tidak ada perubahan perilaku di sisi peramban selain penambahan header keamanan.

Screenshots / Video

Tidak ada perubahan antarmuka pengguna. Pesan error "Format Jenis berkas yang anda unggah berbahaya." tidak lagi muncul pada upload PDF yang valid.

Breaking Changes

None. Nama rule valid_file dan pesan validasi lang/id/validation.php (validation.valid_file) tidak berubah, sehingga seluruh 30+ pemanggil existing (DokumenRequest, ArtikelRequest, GaleriRequest, ProsedurRequest, RegulasiRequest, PotensiRequest, MediaSosialRequest, PengurusRequest, ProfilRequest, SistemKomplainRequest, StoreKomplainRequest, UserRequest, SlideRequest, SinergiProgramRequest, AlbumRequest) tidak perlu disesuaikan.

⚠️ Perilaku yang berubah: file PDF/Office yang kebetulan memuat penanda skrip kini ditolak. Ini merupakan keputusan sadar — konten tersebut tidak pernah dieksekusi, tetapi penolakan dipertahankan sebagai defense in depth. Jika nanti ditemukan dokumen sah yang ditolak karena pola ini, token yang dikecualikan dapat ditambahkan secara spesifik.

Migration Guide

Not required.

References

• Issue [BUG] Validasi valid_file menolak PDF yang valid karena false positive pada isi file #1761: [BUG] Validasi valid_file menolak PDF yang valid karena false positive pada isi file #1761
• Commit asal regex: 15ad14cd — "Buat validasi file valid (Buat validasi file valid #651)", 16 Maret 2023
• OWASP File Upload Cheat Sheet: https://cheatsheetseries.owasp.org/cheatsheets/File_Upload_Cheat_Sheet.html
• MDN X-Content-Type-Options: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Content-Type-Options
• nginx location matching order: https://nginx.org/en/docs/http/ngx_http_core_module.html#location

---

Additional notes:

1. Pola tidak sengaja dihapus: function dan <html dihapus dari daftar global. <html masih terdeteksi untuk MIME markup melalui MARKUP_DANGERS, sedangkan function dihapus permanen karena tidak ada nilai keamanan — string tersebut adalah token pemrograman umum yang wajar muncul di dokumen teknis.

2. Batas 1 MB (SCAN_LIMIT): pemindaian sengaja tidak membaca seluruh file. Penanda skrip pada payload yang disisipkan berada di bagian awal, dan File::get() sebelumnya berpotensi menjadi vektor kehabisan memori pada upload besar. Catatan: untuk PDF, payload PHP praktis tidak pernah dieksekusi, sehingga cakupan 1 MB cukup untuk deteksi dan bukan merupakan boundary keamanan.

3. Perbedaan dari solusi sementara pada issue: usulan di issue hanya memindai image/svg+xml dengan 3 penanda. Pendekatan ini mempertahankan pemindaian universal terhadap marker skrip (termasuk di dalam PDF) dan memperluas daftar penanda SVG dari 3 menjadi 9, karena regex <script saja mudah dilewati lewat onload= atau xlink:href="javascript:".

4. Catatan operasional untuk reviewer: 4 test failure di tests/Feature/FilePreviewFeatureTest.php, tests/Feature/Upload/FileUploadTest.php, dan tests/Feature/ArtikelControllerTest.php bersifat pre-existing dan tidak terkait perubahan ini. Penyebabnya seed TipePotensi kosong pada environment test dan perbedaan URL redirect. Sudah diverifikasi dengan menjalankan suite yang sama pada rilis-dev (hasil identik: 4 failed).

5. Catatan tooling: composer pint tanpa argumen akan mereformat ratusan file di repository ini karena banyak file belum comply dengan konfigurasi Pint saat ini. Untuk PR ini, Pint hanya dijalankan secara scoped pada 6 file yang diubah. Sebaiknya issue terpisah untuk memformat seluruh codebase.

[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 #1765