# Security Vulnerabilities Report

**Last Updated:** December 2025
**Status:** Assessment Findings
**Classification:** Internal Use Only

---

## Executive Summary

A security assessment of the BuyerKiosk authentication system identified **8 critical**, **6 high**, and **5 medium** severity vulnerabilities. Immediate action is recommended for critical issues.

### Risk Summary

| Severity | Count | Immediate Action Required |
|----------|-------|---------------------------|
| 🔴 CRITICAL | 8 | Yes - within 1 week |
| 🟠 HIGH | 6 | Yes - within 1 month |
| 🟡 MEDIUM | 5 | Planned - within 3 months |

---

## Critical Vulnerabilities

### CRIT-001: CSRF Protection Disabled on Login

**Severity:** 🔴 CRITICAL
**CVSS Score:** 8.8 (High)
**Status:** OPEN

**Location:**
- File: `userfrosting/src/BuyerKiosk/Core/Controllers/AccountController.php`
- Lines: 217-221

**Description:**
CSRF token validation is commented out on the login form, allowing attackers to forge login requests.

**Current Code:**
```php
// CSRF token check temporarily disabled
// if (!\NoCSRF::check('csrf_token', $this->_app->request->post())) {
//     $ms->addMessageTranslated("danger", "Invalid or missing CSRF token.");
//     $this->_app->halt(403);
// }
```

**Attack Vector:**
An attacker can create a malicious page that submits login credentials to BuyerKiosk, potentially:
- Logging users into attacker-controlled accounts
- Performing login CSRF attacks
- Session fixation when combined with other vulnerabilities

**Remediation:**
```php
// ENABLE THIS CODE
if (!\NoCSRF::check('csrf_token', $this->_app->request->post())) {
    $ms->addMessageTranslated("danger", "Invalid or missing CSRF token.");
    $this->_app->halt(403);
}
```

**Effort:** 5 minutes
**Risk if Unpatched:** Account takeover, unauthorized access

---

### CRIT-002: SHA1 Password Hashes Accepted

**Severity:** 🔴 CRITICAL
**CVSS Score:** 9.1 (Critical)
**Status:** OPEN

**Location:**
- File: `userfrosting/models/mysql/MySqlUser.php`
- Lines: 358-365

**Description:**
The system still accepts SHA1 password hashes, which are cryptographically broken since 2017.

**Current Code:**
```php
if (Authentication::getPasswordHashType($this->password) == "sha1") {
    $salt = substr($this->password, 0, 25);
    if ($salt . sha1($salt . $password) == $this->password) {
        return true;
    }
}
```

**Attack Vector:**
- SHA1 collisions are computationally feasible
- Fast brute-force attacks (billions of hashes/second on GPU)
- Rainbow table attacks if salt is weak

**Remediation:**
1. Force password reset for all users with SHA1 hashes
2. Or: Keep accepting SHA1 temporarily, but force upgrade on login

```php
// After successful SHA1 verification, force upgrade
if ($this->verifyLegacySha1($password)) {
    $this->password = password_hash($password, PASSWORD_ARGON2ID);
    $this->store();
    return true;
}
```

**Effort:** 2-4 hours
**Risk if Unpatched:** Password compromise, mass account breach

---

### CRIT-003: Password Hash Upgrade Bug

**Severity:** 🔴 CRITICAL
**CVSS Score:** 7.5 (High)
**Status:** OPEN

**Location:**
- File: `userfrosting/models/mysql/MySqlUser.php`
- Lines: 401-410

**Description:**
The automatic password hash upgrade code calls the wrong method, so legacy passwords are never upgraded.

**Current Code:**
```php
if (Authentication::getPasswordHashType($this->password) != "modern") {
    $password_hash = Authentication::getPasswordHashType($password);  // BUG!
    // Should be: Authentication::hashPassword($password)
    if ($password_hash !== null) {
        $this->password = $password_hash;
    }
}
```

**Impact:**
- Users with legacy SHA1 hashes remain vulnerable indefinitely
- No automatic migration to secure hashes

**Remediation:**
```php
if (Authentication::getPasswordHashType($this->password) != "modern") {
    $this->password = Authentication::hashPassword($password);  // FIXED
    $this->store();
}
```

**Effort:** 5 minutes
**Risk if Unpatched:** Perpetual vulnerability for legacy accounts

---

### CRIT-004: API Key Timing Attack Vulnerability

**Severity:** 🔴 CRITICAL
**CVSS Score:** 7.4 (High)
**Status:** OPEN

**Location:**
- File: `userfrosting/models/mysql/MySqlUser.php`
- Line: 390

**Description:**
API key comparison uses `strcmp()` which is vulnerable to timing attacks.

**Current Code:**
```php
if (strcmp($row['key'], $apiKey) == 0 && (int)$row['active'] == 1) {
    return true;
}
```

**Attack Vector:**
Attackers can measure response times to guess API keys character by character.

**Remediation:**
```php
if (hash_equals($row['key'], $apiKey) && (int)$row['active'] == 1) {
    return true;
}
```

**Effort:** 5 minutes
**Risk if Unpatched:** API key compromise, unauthorized API access

---

### CRIT-005: No Brute-Force Protection on Login

**Severity:** 🔴 CRITICAL
**CVSS Score:** 8.1 (High)
**Status:** OPEN

**Location:**
- File: `userfrosting/src/BuyerKiosk/Core/Controllers/AccountController.php`
- Method: `login()`

**Description:**
No rate limiting or account lockout on failed login attempts.

**Attack Vector:**
- Automated credential stuffing attacks
- Password brute-force attacks
- No limit on attempts per IP or per account

**Remediation:**
Implement rate limiting:

```php
// Using Redis for rate limiting
$key = "login_attempts:" . $_SERVER['REMOTE_ADDR'];
$attempts = $redis->incr($key);
$redis->expire($key, 900); // 15 minutes

if ($attempts > 10) {
    $this->_app->halt(429, "Too many login attempts");
}

// Also track per-username
$userKey = "login_attempts:user:" . $username;
$userAttempts = $redis->incr($userKey);
$redis->expire($userKey, 900);

if ($userAttempts > 5) {
    // Lock account temporarily
}
```

**Effort:** 4-8 hours
**Risk if Unpatched:** Account compromise via brute-force

---

### CRIT-006: No Rate Limiting on Password Reset

**Severity:** 🔴 CRITICAL
**CVSS Score:** 7.5 (High)
**Status:** OPEN

**Location:**
- File: `userfrosting/src/BuyerKiosk/Core/Controllers/AccountController.php`
- Method: `forgotPassword()`

**Description:**
No rate limiting on password reset requests.

**Attack Vector:**
- Email bombing (DOS on user's inbox)
- User enumeration via response differences
- Resource exhaustion

**Remediation:**
```php
$key = "password_reset:" . $email;
if ($redis->exists($key)) {
    // Already requested recently
    return "If email exists, reset link sent";
}
$redis->setex($key, 3600, 1); // 1 hour cooldown
```

**Effort:** 2 hours
**Risk if Unpatched:** DOS, user enumeration

---

### CRIT-007: JWT Implementation Broken

**Severity:** 🔴 CRITICAL
**CVSS Score:** 6.5 (Medium-High)
**Status:** OPEN

**Location:**
- File: `userfrosting/src/BuyerKiosk/Core/Controllers/AccountController.php`
- Lines: 296-322

**Description:**
JWT tokens are created but never validated, and have multiple issues:
- 60-second expiration (useless)
- Undefined `$serverName` variable
- Double base64 encoding
- No HttpOnly/Secure flags
- Never validated on requests

**Current Code:**
```php
$expire = $notBefore + 60;  // Only 60 seconds!
$jwt = \JWT::encode($data, $secretKey, 'HS256');
setcookie("pineapple", base64_encode($jwt), time()+259200, '/');
// Cookie lasts 3 days but token only valid 60 seconds!
```

**Impact:**
- False sense of security
- Token can be stolen via XSS (no HttpOnly)
- Dead code creates confusion

**Remediation:**
Either fix properly or remove entirely:

```php
// Option 1: Fix
$expire = $issuedAt + 900; // 15 minutes
setcookie("pineapple", $jwt, [
    'expires' => time() + 900,
    'path' => '/',
    'secure' => true,
    'httponly' => true,
    'samesite' => 'Strict'
]);

// Option 2: Remove dead code
// Delete JWT generation entirely if not used
```

**Effort:** 2-4 hours (fix) or 30 minutes (remove)
**Risk if Unpatched:** Confusion, potential XSS token theft

---

### CRIT-008: Session Cookie Missing Security Flags

**Severity:** 🔴 CRITICAL
**CVSS Score:** 7.1 (High)
**Status:** OPEN

**Location:**
- File: `userfrosting/config-userfrosting.php`

**Description:**
Session cookies lack security attributes:
- No `HttpOnly` flag (XSS can steal session)
- No `Secure` flag (sent over HTTP)
- No `SameSite` attribute (CSRF vulnerable)

**Remediation:**
```php
// Add to config-userfrosting.php
ini_set('session.cookie_httponly', 1);
ini_set('session.cookie_secure', 1);
ini_set('session.cookie_samesite', 'Strict');
ini_set('session.use_strict_mode', 1);
```

**Effort:** 5 minutes
**Risk if Unpatched:** Session hijacking, CSRF

---

## High Severity Vulnerabilities

### HIGH-001: No Account Lockout Mechanism

**Severity:** 🟠 HIGH
**Status:** OPEN

**Description:**
No mechanism to lock accounts after repeated failed login attempts.

**Remediation:**
Add `failed_login_attempts` and `locked_until` columns to user table.

**Effort:** 4 hours

---

### HIGH-002: Password Reset Tokens Don't Expire

**Severity:** 🟠 HIGH
**Status:** OPEN

**Location:**
- File: `userfrosting/src/BuyerKiosk/Core/Controllers/AccountController.php`

**Description:**
Password reset tokens have no explicit expiration check.

**Remediation:**
Check `lost_password_timestamp` and reject if > 1 hour old.

**Effort:** 1 hour

---

### HIGH-003: RememberMe Tokens Unencrypted in Database

**Severity:** 🟠 HIGH
**Status:** OPEN

**Description:**
Persistent tokens stored in plaintext in `uf_user_rememberme`.

**Remediation:**
Hash tokens before storage, compare with `hash_equals()`.

**Effort:** 4 hours

---

### HIGH-004: No MFA/2FA Support

**Severity:** 🟠 HIGH
**Status:** OPEN

**Description:**
Single-factor authentication only. No TOTP, WebAuthn, or other second factor.

**Remediation:**
Implement TOTP support as optional second factor.

**Effort:** 2-3 weeks

---

### HIGH-005: Fragile Hash Type Detection

**Severity:** 🟠 HIGH
**Status:** OPEN

**Location:**
- File: `userfrosting/auth/Authentication.php`
- Lines: 7-14

**Description:**
Hash type detected by string length (65 chars = SHA1).

**Current Code:**
```php
if (strlen($password) == 65) {
    return "sha1";
}
return "modern";
```

**Remediation:**
Use explicit hash prefixes or store hash type in database.

**Effort:** 2-4 hours

---

### HIGH-006: Username Enumeration Possible

**Severity:** 🟠 HIGH
**Status:** OPEN

**Description:**
Different error messages for "user not found" vs "wrong password" allow username enumeration.

**Remediation:**
Use generic "Invalid credentials" message for all failures.

**Effort:** 1 hour

---

## Medium Severity Vulnerabilities

### MED-001: No Login Audit Logging

**Severity:** 🟡 MEDIUM
**Status:** OPEN

**Description:**
Successful and failed logins not logged for forensic analysis.

**Effort:** 4 hours

---

### MED-002: No Session Management UI

**Severity:** 🟡 MEDIUM
**Status:** OPEN

**Description:**
Users cannot view or revoke active sessions.

**Effort:** 1-2 weeks

---

### MED-003: BCrypt Cost Factor Too Low

**Severity:** 🟡 MEDIUM
**Status:** OPEN

**Description:**
Using default cost 10. Should be 12+ for modern hardware.

**Effort:** 1 hour

---

### MED-004: No Password Complexity Requirements

**Severity:** 🟡 MEDIUM
**Status:** OPEN

**Description:**
No minimum length or complexity requirements enforced.

**Effort:** 2 hours

---

### MED-005: Error Logging to error_log

**Severity:** 🟡 MEDIUM
**Status:** OPEN

**Description:**
Security events logged to PHP error_log instead of structured audit log.

**Effort:** 8 hours

---

## Remediation Priority

### Immediate (This Week)

| ID | Issue | Effort |
|----|-------|--------|
| CRIT-001 | Enable CSRF on login | 5 min |
| CRIT-003 | Fix hash upgrade bug | 5 min |
| CRIT-004 | Fix API key timing attack | 5 min |
| CRIT-008 | Add session cookie flags | 5 min |

### Short Term (This Month)

| ID | Issue | Effort |
|----|-------|--------|
| CRIT-002 | Handle SHA1 passwords | 2-4 hours |
| CRIT-005 | Add login rate limiting | 4-8 hours |
| CRIT-006 | Add password reset rate limiting | 2 hours |
| CRIT-007 | Fix or remove JWT | 2-4 hours |
| HIGH-001 | Add account lockout | 4 hours |
| HIGH-002 | Add reset token expiry | 1 hour |

### Medium Term (This Quarter)

| ID | Issue | Effort |
|----|-------|--------|
| HIGH-003 | Encrypt RememberMe tokens | 4 hours |
| HIGH-004 | Implement MFA | 2-3 weeks |
| HIGH-005 | Fix hash detection | 2-4 hours |
| HIGH-006 | Fix username enumeration | 1 hour |
| MED-001 | Add audit logging | 4 hours |
| MED-002 | Session management UI | 1-2 weeks |

---

## Quick Wins (Copy-Paste Fixes)

### Fix 1: Enable CSRF

**File:** `AccountController.php:217-221`
```php
// UNCOMMENT THIS:
if (!\NoCSRF::check('csrf_token', $this->_app->request->post())) {
    $ms->addMessageTranslated("danger", "Invalid or missing CSRF token.");
    $this->_app->halt(403);
}
```

### Fix 2: Hash Upgrade Bug

**File:** `MySqlUser.php:404`
```php
// CHANGE THIS:
$password_hash = Authentication::getPasswordHashType($password);
// TO THIS:
$password_hash = Authentication::hashPassword($password);
```

### Fix 3: API Key Timing Attack

**File:** `MySqlUser.php:390`
```php
// CHANGE THIS:
if (strcmp($row['key'], $apiKey) == 0 ...
// TO THIS:
if (hash_equals($row['key'], $apiKey) ...
```

### Fix 4: Session Cookie Flags

**File:** `config-userfrosting.php` (add at top)
```php
ini_set('session.cookie_httponly', 1);
ini_set('session.cookie_secure', 1);
ini_set('session.cookie_samesite', 'Strict');
```

---

## Compliance Considerations

### PCI-DSS (if handling payment data)

| Requirement | Status | Issue |
|-------------|--------|-------|
| 8.1.6 Lockout after 6 attempts | ❌ FAIL | No lockout |
| 8.1.8 Session timeout | ⚠️ PARTIAL | 24h too long |
| 8.2.3 Strong passwords | ❌ FAIL | No requirements |
| 8.2.5 Password history | ❌ FAIL | Not tracked |
| 8.4 Multi-factor auth | ❌ FAIL | Not implemented |

### SOC 2

| Criteria | Status | Issue |
|----------|--------|-------|
| CC6.1 Access control | ⚠️ PARTIAL | No MFA |
| CC6.6 Security events | ❌ FAIL | No audit log |
| CC6.7 Authorized users | ⚠️ PARTIAL | No session management |

---

## Related Documentation

- [Authentication Architecture](./authentication-architecture.md)
- [Unified Users Specification](../specs/unified-users-modern-auth-spec.md)