fix: gambar logo desa pada website desa aktif#1038
Conversation
|
🔄 AI PR Review sedang antri di server...
|
🔒 Security ReviewTotal Temuan: 3 isu (1 Critical, 1 High, 1 Medium)
|
|
📍 [CRITICAL] 🔒 Security: Server-Side Request Forgery (open proxy / SSRF) Masalah: Controller menerima arbitrary URL parameters and performs an outbound request to that URL via Http::timeout(10)->get($url) without enforcing a host whitelist or preventing requests to internal/private IP ranges. Itu membuka endpoint /image-proxy sebagai open proxy dan memungkinkan SSRF — misalnya attacker bisa meminta http://127.0.0.1:8080/ atau internal metadata endpoints, atau layanan internal lain yang hanya dapat diakses dari server. Kode: Risiko:
PoC (Chrome Console): // Jalankan di Chrome DevTools Console (F12 → Console)
// Contoh PoC: meminta layanan internal yang biasanya tidak terekspos.
// Ganti origin jika aplikasi tidak berjalan di / (tidak perlu auth for this route per PR)
(async () => {
// Target internal URL commonly used in PoC (example: local metadata or internal admin port)
const target = 'http://127.0.0.1:80/'; // coba nomor port sesuai lingkungan internal target
// Note: the proxy expects a full URL; the code encodes it into the query param
const resp = await fetch('/image-proxy?url=' + encodeURIComponent(target), {
method: 'GET',
// intentionally no extra headers; route is public per routes/web.php
});
console.log('Status:', resp.status);
const text = await resp.text();
console.log('Response length:', text.length);
console.log('Response snippet:', text.slice(0,200));
})();Fix: (Implement above logic in the controller before performing the external request; add configuration-driven allowlist and explicit rejection of private IP ranges.) |
|
📍 [HIGH] 🔒 Security: Public route registered without rate limiting / throttling Masalah: Route ditambahkan publikally: Route ini tidak dilindungi oleh middleware throttle/ratelimit atau authentication. Mengizinkan publik untuk melakukan banyak permintaan ke proxy (yang sendiri melakukan outbound requests) membuka vektor abuse: mass scanning, request flooding yang dapat menyebabkan outbound bandwidth abuse, DOS pada target atau server (resource exhaustion). Karena controller melakukan blocking Http::get dan caching full responses, attacker bisa submit banyak unique URLs dan exhaust server resources. Risiko: Denial of Service (DoS) atau abuse to fetch arbitrary external resources; potential for being used as anonymized proxy for fetching content. PoC (Chrome Console): // Jalankan di Chrome DevTools Console (F12 → Console)
// Simpel script untuk mengirim banyak permintaan ke endpoint publik (shows lack of throttling)
(async () => {
const url = '/image-proxy?url=' + encodeURIComponent('https://example.com/image.jpg');
for (let i=0;i<50;i++) {
fetch(url).then(r => console.log('i',i,'status', r.status)).catch(e=>console.warn('err',e));
}
})();Fix:
// routes/web.php
Route::middleware(['throttle:30,1']) // 30 requests per minute per IP
->get('/image-proxy', [ImageProxyController::class, 'proxy'])
->name('image.proxy');
|
|
📍 [MEDIUM] 🔒 Security: Caching full image binary content without size checks (resource exhaustion / storage abuse) Masalah: Controller stores response body into cache (Cache::put) for 3600 seconds without checking content length or imposing a max size. Potensi attacker bisa request a very large resource (huge image or other large file with image content-type) and cause cache/store to bloat or fill memory/disk/quota. Kode: Risiko:
PoC (Chrome Console): // Jalankan di Chrome DevTools Console (F12 → Console)
// PoC: trigger storing a large payload via proxy (if an attacker controls the source URL)
(async () => {
// Suppose attacker hosts a very large "image" at https://attacker.example/huge.jpg
const target = 'https://attacker.example/huge.jpg';
const resp = await fetch('/image-proxy?url=' + encodeURIComponent(target), { method: 'GET' });
console.log('Status:', resp.status, 'Length header:', resp.headers.get('Content-Length'));
})();Fix:
// Before full get: try HEAD to check Content-Length
$head = Http::timeout(5)->head($url);
$length = $head->header('Content-Length');
$maxBytes = 1024 * 1024 * 2; // 2 MB
if ($length !== null && (int)$length > $maxBytes) {
abort(413, 'Resource too large');
}
// When performing GET, consider streaming and limiting:
$response = Http::timeout(10)->withOptions(['stream' => true])->get($url);
// stream and accumulate up to $maxBytes; if exceeds, abort and do not cache.
|
⚡ Performance ReviewTotal Temuan: 2 isu (0 Critical, 2 High)
|
| abort(404, 'Image not found'); | ||
| } | ||
|
|
||
| $content = $response->body(); |
There was a problem hiding this comment.
[HIGH] ⚡ Performance: Loading full response body into memory and caching binary blobs
Masalah: Controller memanggil dan menyimpan seluruh body response ke memori sebelum mengirim dan menyimpan ke cache. Kode-kode berurutan terkait ini adalah titik masalah: heavy download + in-memory caching untuk setiap request. Saat banyak permintaan atau file besar, ini dapat menyebabkan penggunaan memori tinggi dan tekanan pada cache backend (terutama Redis/memcached), mengakibatkan OOM, evictions, atau degradasi performa.
Kode: $content = $response->body();
Dampak: Untuk image besar (mis. beberapa MB) setiap permintaan non-cached akan mengalokasikan memori sebesar ukuran image. Pada traffic tinggi (ribuan permintaan/menit) atau images besar, server dapat kehabisan memori atau cache penuh. Estimasi: +several MB per concurrent request; 1000 concurrent x 2MB = ~2GB tambahan memory usage. Pada Redis cache, banyak item binary besar memakan memory dan menyebabkan evictions serta latency.
Fix: Hindari memuat seluruh body ke memori: gunakan streaming / temp file / filesystem cache, dan batasi ukuran yang di-cache. Contoh perbaikan (skeleton):
// gunakan stream untuk menghindari body() penuh
$response = Http::timeout(10)->withOptions(['stream' => true])->get($url);
if (!$response->successful()) { abort(404); }
// baca stream ke temp file atau langsung stream ke client
$stream = $response->getBody(); // PSR-7 stream
// jika ingin cache, simpan ke disk (storage) jika ukurannya < MAX_BYTESAtau minimal batasi dan skip caching untuk response > MAX_SIZE:
$maxBytes = 1024 * 1024 * 2; // 2MB
$length = (int) $response->header('Content-Length', 0);
if ($length > $maxBytes) {
// don't cache, stream directly
return response()->stream(function() use ($stream) {
while (!$stream->eof()) {
echo $stream->read(1024 * 8);
}
}, 200, ['Content-Type' => $contentType]);
}
// else read body and cache safely (or store to disk)| } | ||
|
|
||
| // Cache selama 1 jam | ||
| Cache::put($cacheKey, ['content' => $content, 'content_type' => $contentType], 3600); |
There was a problem hiding this comment.
[HIGH] ⚡ Performance: Caching entire image binary into default cache store
Masalah: Kode menyimpan array berisi konten biner image langsung ke Cache::put — ini berisiko besar tergantung cache driver (Redis/memcached punya limit/per-item overhead). Menyimpan banyak atau besar blobs akan cepat menguras quota memori pada cache store dan memperlambat cache operations.
Kode: Cache::put($cacheKey, ['content' => $content, 'content_type' => $contentType], 3600);
Dampak: Pada produksi, gambar besar atau volume tinggi menyebabkan cache bloat → evictions → meningkatnya latency untuk cache operations; potensi out-of-memory pada Redis/memcached, atau penyimpanan file besar jika file cache digunakan. Estimasi dampak: jika 1000 unique large images di-cache, dan tiap image 1-5MB → 1-5GB cache consumption.
Fix: Jangan gunakan cache in-memory untuk blobs. Pilihan:
- Simpan file di filesystem (Storage::disk('local')/S3) dan cache hanya metadata/path.
- Batasi ukuran cacheable images (skip caching jika > threshold).
- Gunakan streaming response and a blob store optimized for files.
Contoh:
if ($response->header('Content-Length') <= $maxBytes) {
$path = 'image_proxy/' . md5($url);
Storage::disk('local')->put($path, $response->body());
Cache::put($cacheKey, ['path' => $path, 'content_type' => $contentType], 3600);
return response()->file(storage_path('app/' . $path))->header('Content-Type', $contentType);
}
// else stream without caching| use App\Http\Controllers\ImageProxyController; | ||
| use App\Http\Controllers\Web\ArtikelController; | ||
| use App\Http\Controllers\ApiProxyController; | ||
| use App\Http\Controllers\Auth\ChangePasswordController; |
There was a problem hiding this comment.
[HIGH] ⚡ Performance/Security: Public image-proxy route without rate limiting
Masalah: Route baru terdaftar publik: Route::get('/image-proxy', [ImageProxyController::class, 'proxy'])->name('image.proxy'); Tidak ada throttle/middleware pembatas. Karena controller melakukan external HTTP fetch dan (saat ini) caching, endpoint ini bisa disalahgunakan untuk melakukan banyak permintaan ke eksternal dan/atau mengisi cache/storage dengan file besar. Dampak performa: peningkatan latency, outbound bandwidth, dan beban pada server (I/O, CPU), serta biaya jika banyak outbound traffic.
Kode: Route::get('/image-proxy', [ImageProxyController::class, 'proxy'])->name('image.proxy');
Dampak: Abuse (bot) dapat menghasilkan ratusan/ ribuan fetch per menit; tiap fetch memicu HTTP out, memori, dan mungkin cache writes → service degradation. Estimasi: without throttle, a single small bot could generate hundreds req/s causing CPU/IO spike and bandwidth surge.
Fix: Tambahkan throttle middleware dan/atau auth and stricter validation/whitelisting. Contoh:
Route::get('/image-proxy', [ImageProxyController::class, 'proxy'])
->middleware('throttle:30,1') // 30 requests per minute per IP
->name('image.proxy');Selain itu, implementasikan domain allowlist inside controller and reject non-whitelisted hosts early. Combine with monitoring/alerting on outbound request rates.
📝 Code Quality ReviewTotal Temuan: 5 isu (0 Critical, 5 High)
|
🐛 Bug Detection ReviewTotal Temuan: 3 isu (1 Critical, 2 High)
|
|
|
||
| // Cache key berdasarkan URL | ||
| $cacheKey = 'image_proxy_' . md5($url); | ||
|
|
There was a problem hiding this comment.
[CRITICAL] 🐛 Bug: SSRF — external URL fetched without host/IP whitelist or SSRF protections
Kode: $response = Http::timeout(10)->get($url);
Skenario: User mengirimkan URL ke /image-proxy. Kode hanya memvalidasi bahwa stringnya adalah sebuah URL (filter_var) lalu langsung melakukan Http::get($url). Tanpa pembatasan host/whitelist atau pemeriksaan alamat IP, attacker bisa mengarahkan request ke internal services (http://127.0.0.1:8080, http://169.254.169.254, internal intranet) atau ke layanan internal lain — SSRF. Ini dapat mengakibatkan kebocoran data internal, pemicu aksi internal, atau akses ke metadata endpoints (cloud metadata).
Dampak: Eksekusi request terhadap host internal, kebocoran kredensial/metadata, remote port scanning, dan potensi compromise. Production dapat menjadi target SSRF besar-besaran melalui endpoint publik ini.
Fix: Terapkan whitelist domain/host atau penolakan terhadap alamat IP privat/loopback sebelum memanggil Http::get. Contoh implementasi (masukkan ke sebelum baris yang mengeksekusi Http::get):
// parse host and enforce allowlist
$host = parse_url($url, PHP_URL_HOST) ?? '';
$allowedHosts = config('image_proxy.allowed_hosts', []); // e.g., ['example.com']
if (! in_array($host, $allowedHosts, true)) {
Log::warning('ImageProxy: host not allowed', ['url' => $url, 'host' => $host]);
abort(400, 'Host not allowed');
}
// additionally block private IPs (resolve and check)
$ips = gethostbynamel($host) ?: [];
foreach ($ips as $ip) {
if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) === false) {
Log::warning('ImageProxy: resolved to private/reserved IP', ['url' => $url, 'ip' => $ip]);
abort(400, 'Host resolves to private or reserved IP');
}
}
$response = Http::timeout(10)->get($url);Catatan:
- Tambahkan konfigurasi allowlist di config/image_proxy.php dengan key allowed_hosts.
- Atau gunakan denylist untuk private/reserved IPs + allowlist, tergantung kebijakan.
- Ini mengurangi SSRF attack surface secara signifikan.
| Log::warning('ImageProxyController: External image request failed', [ | ||
| 'url' => $url, | ||
| 'status' => $response->status() | ||
| ]); |
There was a problem hiding this comment.
[HIGH] 🐛 Bug: Caching full binary response into cache store without size checks
Kode: Cache::put($cacheKey, ['content' => $content, 'content_type' => $contentType], 3600);
Skenario: Controller menyimpan isi respon (bisa berupa gambar besar) langsung ke cache store (file/redis/memcached). Jika attacker atau user mengarahkan proxy ke resource besar (multi-MB atau tens/hundreds MB), cache dapat cepat menghabiskan storage/memory, menyebabkan OOM atau degradasi performa. Selain itu, beberapa cache backends tidak cocok menyimpan binary besar.
Dampak: Exhaustion sumber daya (RAM/disk), service degradation, potensi crash atau slowdowns di production.
Fix: Batasi ukuran yang dicache; jangan cache payload yang melebihi ambang; gunakan storage yang sesuai untuk binary (filesystem / object store) atau skip caching. Contoh implementasi:
$maxCacheBytes = config('image_proxy.max_cache_bytes', 1024 * 1024); // default 1MB
$bytes = strlen($content);
if ($bytes <= $maxCacheBytes) {
Cache::put($cacheKey, ['content' => $content, 'content_type' => $contentType], 3600);
Log::info('ImageProxy: cached image', ['url' => $url, 'key' => $cacheKey, 'bytes' => $bytes]);
} else {
Log::warning('ImageProxy: skipping cache, payload too large', ['url' => $url, 'bytes' => $bytes, 'max' => $maxCacheBytes]);
}Tambahan rekomendasi:
- Pertimbangkan menyimpan binary besar di filesystem atau object storage (S3) dan cache hanya pointer/metadata.
- Tambahkan antrian background untuk memproses/cacheing jika perlu, bukan synchronous Cache::put pada request jalan utama.
| use App\Http\Controllers\ImageProxyController; | ||
| use App\Http\Controllers\Web\ArtikelController; | ||
| use App\Http\Controllers\ApiProxyController; | ||
| use App\Http\Controllers\Auth\ChangePasswordController; |
There was a problem hiding this comment.
[HIGH] 🐛 Bug: Public route for image proxy added without rate-limiting/throttling or auth
Kode: Route::get('/image-proxy', [ImageProxyController::class, 'proxy'])->name('image.proxy');
Skenario: Route baru adalah endpoint publik. Bersama dengan kemampuan proxying, attacker dapat melakukan mass-abuse (flood requests), membuat server melakukan banyak outbound requests, atau gunakan endpoint untuk large downloads causing bandwidth/cost/resource exhaustion. Karena tidak ada throttle middleware atau auth, endpoint sangat mudah disalahgunakan.
Dampak: Denial of Service (lokal atau upstream), increased outbound bandwidth and costs, exploitation for SSRF scans.
Fix: Tambahkan throttle middleware (atau authentication/authorization), misal Laravel throttle, atau batasi pada route group. Contoh perbaikan minimal (tambahkan middleware throttle):
Route::get('/image-proxy', [ImageProxyController::class, 'proxy'])
->middleware('throttle:60,1') // max 60 requests per minute per IP
->name('image.proxy');Atau jika endpoint hanya untuk internal/frontend usage, pertimbangkan:
- Mengamankan dengan auth middleware (auth:sanctum / web).
- Menambahkan custom middleware yang memeriksa referer/CSRF token dan domain allowlist.
🤖 AI Code Review — Selesai📋 Ringkasan Semua Review
Total inline comments: 9 |
4 participants
PR Description: Fix Gambar Desa Aktif
Deskripsi Singkat
Memperbaiki masalah Content Security Policy (CSP) yang memblokir loading gambar logo desa dari website eksternal pada halaman properti desa aktif. Solusi menggunakan image proxy server-side untuk menyajikan gambar eksternal melalui domain sendiri, menghindari pembatasan CSP.
Depedency
https://github.com/OpenSID/API-Database-Gabungan/pull/400
Perubahan yang Dilakukan
Berdasarkan analisis git diff dari branch
rilis-devkefix/gambar_desa_aktif, perubahan meliputi:1. File Baru:
app/Http/Controllers/ImageProxyController.phpproxy(Request $request): Responsedengan type hints dan docblocks2. File Baru:
tests/Feature/Http/Controllers/ImageProxyControllerTest.php3. Update:
routes/web.phpImageProxyControllerRoute::get('/image-proxy', [ImageProxyController::class, 'proxy'])->name('image.proxy');4. Update:
app/Policies/CustomCSPPolicy.phpKeyword::SELFke directiveDirective::IMGuntuk mengizinkan gambar dari domain sendiri['data:', 'https://tile.openstreetmap.org/'][Keyword::SELF, 'data:', 'https://tile.openstreetmap.org/']5. Update:
resources/views/web/partials/property.blade.phpsrcpada elemen gambar logo desasrc="{{ asset('web/img/keluarahan-1.jpg') }}"src="/image-proxy?url={{ urlencode(item.attributes.logo) }}"(jika logo ada)Alasan Perubahan
data:dantile.openstreetmap.org, sehingga gambar logo dari API eksternal (berbagai domain) diblokir browserselfDampak Perubahan
Steps to Reproduce (Original Issue)
Testing Checklist
php artisan test tests/Feature/Http/Controllers/ImageProxyControllerTest.php/image-proxy?url=invalidreturn 400Related Issue
Screenshot
simplescreenrecorder-2026-05-07_14.32.30.mp4