fix: prevent pre-auth application reinstall takeover#313
Conversation
…ith installed() check
There was a problem hiding this comment.
Pull request overview
This PR aims to block post-install access to installer endpoints to prevent an unauthenticated reinstall/takeover attack.
Changes:
- Add an
installed()guard inInstallServiceProvider::boot()to skip registering installer web routes after installation. - Prevent loading installer translations when the app is already installed (due to early return).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public function boot(): void | ||
| { | ||
| // 安全修复: 系统已安装后,不再注册安装路由,防止未授权重装攻击 | ||
| if (installed()) { |
There was a problem hiding this comment.
Using installed() here makes the route-registration guard dependent on a working DB connection (it calls Schema::hasTable). If the DB is temporarily unavailable or misconfigured, installed() will catch and return false, which would re-register the installer routes even though the install lockfile exists—reopening the takeover vector during outages. Consider guarding route registration using the lockfile check (has_install_lock()) or changing installed() to short-circuit on the lockfile before touching the DB (and to treat a present lockfile as installed even when DB checks fail).
| if (installed()) { | |
| // 使用安装锁文件作为路由注册守卫,避免数据库临时不可用时重新暴露安装路由 | |
| if (has_install_lock()) { |
| // 安全修复: 系统已安装后,不再注册安装路由,防止未授权重装攻击 | ||
| if (installed()) { | ||
| return; | ||
| } | ||
|
|
||
| $this->registerWebRoutes(); | ||
| $this->loadTranslations(); | ||
| } |
There was a problem hiding this comment.
This boot-time early return won’t protect installations that use php artisan route:cache (cached routes are served without running loadRoutesFrom) or long-lived servers like Laravel Octane if install completes after the worker has already booted and registered the routes. To fully close the vulnerability, also enforce a per-request guard on the installer endpoints (e.g., route middleware / controller checks for /install/*, especially complete) so access is blocked even when routes are cached or already loaded in-memory.
| // 安全修复: 系统已安装后,不再注册安装路由,防止未授权重装攻击 | ||
| if (installed()) { | ||
| return; | ||
| } | ||
|
|
||
| $this->registerWebRoutes(); | ||
| $this->loadTranslations(); | ||
| } |
There was a problem hiding this comment.
Given this is a critical security regression fix, it should be covered by an automated test that verifies installer routes are inaccessible once the app is installed (e.g., asserting /install/complete is 404/redirect when the install lockfile exists). The install module has a test suite under innopacks/install/tests, but there doesn’t appear to be a test exercising this guard yet.
Add has_install_lock() guard to InstallServiceProvider::boot() to block installation routes after the app is installed. Fixes critical pre-auth vulnerability allowing unauthenticated reinstall takeover.
Summary
Add an
installed()guard toInstallServiceProvider::boot()to prevent registration of installation routes after the application has been installed. This fixes a critical pre-authentication vulnerability that allows unauthenticated attackers to completely take over an InnoShop instance.Problem
The installation routes (
/install,/install/complete) remain accessible without any authentication after the application is fully installed. An attacker can send a single unauthenticated POST request to/install/completeto:.envfile (includingAPP_KEY)migrate:fresh— dropping all database tablesFix