📋 Laravel Technical Audit

📌 Overview

Overall risk classification: 🔴 Critical

The codebase demonstrates severe security and architectural deficiencies. The most alarming issues include a Global IDOR (Insecure Direct Object Reference) in the account scoping middleware, which allows any authenticated user to impersonate any other account, and Non-Atomic Financial Checks, which expose the system to multi-user race conditions and potential financial drain. Additionally, the presence of hardcoded sensitive credentials and weak authentication factors (4-digit PINs for financial withdrawals) places the entire platform at high risk of compromise.

---

🚨 Critical Issues

🔴 Issue 1: Global Account Impersonation (IDOR)

• Type: Security / Broken Access Control
• Severity: Critical
• Affected file: app/Http/Middleware/SetActiveAccountMiddleware.php
• Code snippet:

public function handle(Request $request, Closure $next): Response {
    $accountHeader = $request->header('account') ?? $request->header('X-Customer-Id');
    // ...
    } else {
        $account = Account::find($accountHeader); // <--- NO PERMISSION CHECK
        if (!$account) { throw new ModelNotFoundException('Conta não encontrada.', 404); }
    }
    $request->attributes->set('account', $account);
    return $next($request);
}

• Technical description: The middleware trusts the account header value without verifying if the authenticated user belongs to or has permissions for that specific account.
• Actual impact: Any logged-in user can access data and perform actions on behalf of ANY other account in the system simply by changing the HTTP header.
• Possible exploit: An attacker can enumerate account IDs and scrape balances, client data, and initiate transactions across the entire platform.
• Recommended fix: Verify the mapping between the user and the requested account (e.g., via the user_accounts pivot table).
• Corrected code:

$account = Auth::user()->accounts()->where('accounts.id', $accountHeader)->firstOrFail();

🔴 Issue 2: Financial Race Condition (Balance Exhaustion)

• Type: Logic / Concurrency
• Severity: Critical
• Affected file: app/Http/Controllers/QuickPay/SicrediController.php (and WithdrawController.php)
• Code snippet:

$balance = $accout->balance() - $txa;
if ($balance < $request->mount) {
    return response()->json(['message' => 'Saldo insuficiente.'], 400);
}
// ... API CALL TO PAY ...

• Technical description: The balance check is not atomic. If two identical requests are sent simultaneously, both could read the "sufficient" balance before either results in a debit, leading to a negative balance and loss of funds.
• Actual impact: Financial loss due to overdraft/double-spending.
• Possible exploit: Using tools like burp intruder or simple scripts to send concurrent withdrawal requests.
• Recommended fix: Use database-level negative balance constraints and pessimistic locking (lockForUpdate).
• Corrected code:

DB::transaction(function () use ($accout, $request) {
    $balanceRecord = Balance::where('account_id', $accout->id)->lockForUpdate()->first();
    if ($balanceRecord->balance < $request->mount + $accout->tax_pix_out) {
        throw new \Exception("Saldo insuficiente.");
    }
    // Record transaction...
});

🔴 Issue 3: Hardcoded Application Secrets

• Type: Security
• Severity: Critical
• Affected file: app/Http/Controllers/QuickPay/SicrediController.php
• Code snippet:

"client_id" => "multipag-fastgivrmoneyip-client",
"client_secret" => "y5EU7QV3H6YWo9hzLJUFgvWwxfi5f9t6",

• Technical description: Production credentials for the Sicredi payment gateway are hardcoded in the repository.
• Actual impact: Exposure of credentials via source code access. Full ability for an attacker to manipulate bank payments.
• Recommended fix: Use .env and config() helpers.

🔴 Issue 4: Missing Ownership Check in Withdrawal Confirmation

• Type: Security / IDOR
• Severity: Critical
• Affected file: app/Http/Controllers/WithdrawController.php
• Code snippet:

public function endWithdraw(Request $request) {
    // ...
    $pix = PixWithdraw::whereTxid($request->txid)->whereStatus(0)->first();
    if ($pix === null) { ... }
    // ... continues to confirm based on the TXID only ...
}

• Technical description: The endWithdraw method retrieves the withdrawal record by txid but does not scope it to the currently authenticated account.
• Actual impact: An attacker knowing a txid (possibly via enumeration or sniffing) could confirm a withdrawal for another account.
• Recommended fix: Scope the query to the account in the request attributes.
• Corrected code:

$account = $request->attributes->get('account');
$pix = PixWithdraw::where('txid', $request->txid)
    ->where('account_id', $account->id)
    ->where('status', 0)
    ->firstOrFail();

---

⚠ Moderate Issues

🟡 Use of env() outside config files

• Type: Best Practice / Performance
• Severity: Moderate
• Description: env() is called directly in multiple controllers (e.g. BoletoController). If config:cache is executed, these calls will always return null.
• Impact: Application fails mysteriously in production.
• Fix: Map values in config/*.php and use config('app.url').

🟡 Low Entropy for Financial Codes

• Type: Security
• Severity: Moderate
• Description: Withdrawal codes are 4-digit numeric (rand(1000, 9999)).
• Impact: Vulnerable to brute-force attacks within the 4-minute expiration window.
• Fix: Use Str::random(6) or 6-digit codes with strict rate-limiting.

🟡 Mass Assignment Risk on User

• Type: Security
• Severity: Moderate
• Description: status is in $fillable. If a profile edit endpoint passes request()->all() to $user->update(), a user can reactivate themselves or change internal statuses.

---

ℹ Minor Issues

🔵 Redundant Queries in Observers

• Type: Performance
• Description: TransactionObserver calls $transaction->update() inside the created hook. This triggers a second query immediately after every insert.
• Fix: Use the creating hook to modify attributes before the insert.

---

🔒 OWASP Analysis

• A01:2021-Broken Access Control: CRITICAL. Global IDOR in Middleware.
• A02:2021-Cryptographic Failures: CRITICAL. Hardcoded Client Secret.
• A07:2021-Identification and Authentication Failures: HIGH. Low entropy PINs.
• Mass Assignment: MEDIUM. Sensitive fields in $fillable models.

---

⚡ Performance Bottlenecks

1. Inefficient Balance View: vw_balance likely sums all transactions on the fly. As the database grows to millions of rows, checking balance will stop scaling. Suggestion: Use a cached balance column in the accounts or balances table, updated via increments.
2. N+1 Queries: Potential in InvoiceController when loading pix and boleto relations inside the ChargeResource without eager loading in the controller.
3. Memory Limits: ini_set('memory_limit', '-1') in InvoiceController is a "code smell" indicating poor query optimization in exports.

---

🧠 Architectural Recommendations

1. Service Layer: Extract financial logic from SicrediController and WithdrawController into specialized Services.
2. Transaction Scoping: Moving mapping checks into Middleware or dedicated Trait for all Models.
3. Pessimistic Locking: Crucial for all financial debit operations.

---

🧪 Testing Recommendations

1. Concurrency Tests: Use laravel/pint or custom scripts to test the balance check under load.
2. IDOR Tests: Create Feature tests that attempt to access Account B's data using Account A's token + header.
3. Mocking: Use Http::fake() to mock the Sicredi gateway respond.

---

✅ Final Checklist

• [ ] Secure mass assignment (❌ - status exposed)
• [ ] Correct authorization (❌ - Critical IDOR in Middleware)
• [ ] No N+1 (⚠️ - Potential in resources)
• [ ] No SQL Injection (✅ - Mostly using Eloquent/Bindings)
• [ ] Correct transactions (❌ - Missing atomic locks)
• [ ] Testable code (⚠️ - Coupled controllers)
• [ ] Sustainable architecture (⚠️ - God controllers)