# Security and Bug Audit Report
**Date**: 2026-04-29  
**Scope**: PHP files in admin/, _app/api/, _app/scripts/, includes/  
**Repository**: COGS Australia

## CRITICAL Severity Findings

### 1. SQL Injection in Dynamic Table Queries
- **Severity**: CRITICAL
- **File**: `admin/includes/ops_workflow.php:91`
- **Description**: Direct table name concatenation in SQL query without validation
- **Why it matters**: Attackers could inject arbitrary SQL commands through the table parameter. This could lead to data theft, modification, or deletion.
- **Suggested fix**: Implement a whitelist of allowed table names or use proper escaping with `$pdo->quote()` for identifiers.

### 2. Exposed Database Test Script
- **Severity**: CRITICAL
- **File**: `api/dbtest.php`
- **Description**: Database connection test script accessible without authentication
- **Why it matters**: Exposes database configuration details and connection status to anyone. This information aids attackers in planning database attacks.
- **Suggested fix**: Remove this file immediately from production environments.

### 3. Unprotected Admin Tool
- **Severity**: CRITICAL
- **File**: `resend_admin.php`
- **Description**: Admin email resend tool accessible without any authentication
- **Why it matters**: Allows anyone to send emails on behalf of the system and view member data. Could be used for spam or phishing attacks.
- **Suggested fix**: Delete immediately after use as noted in file comments; never deploy to production.

## HIGH Severity Findings

### 4. Debug Mode Enabled in Production
- **Severity**: HIGH
- **File**: `admin/index.php:3-4`
- **Description**: PHP error display enabled with E_ALL reporting
- **Why it matters**: Exposes internal file paths, database structure, and implementation details in error messages. Aids attackers in understanding the system.
- **Suggested fix**: Set `display_errors = 0` and use proper error logging instead.

### 5. Missing CSRF Protection
- **Severity**: HIGH
- **File**: Multiple admin forms and endpoints
- **Description**: No CSRF token validation on state-changing operations
- **Why it matters**: Attackers can trick authenticated admins into performing unwanted actions. Could lead to unauthorized data modifications.
- **Suggested fix**: Implement CSRF tokens on all forms and validate on submission.

### 6. Admin Bootstrap Lacks Rate Limiting
- **Severity**: HIGH
- **File**: `_app/api/routes/auth.php:298-345`
- **Description**: Admin bootstrap endpoint has no rate limiting or brute force protection
- **Why it matters**: Attackers could attempt unlimited guesses of the bootstrap token. Even with a strong token, this poses unnecessary risk.
- **Suggested fix**: Add rate limiting (e.g., 3 attempts per hour), IP restrictions, and comprehensive logging.

### 7. SQL Injection via Dynamic ORDER BY
- **Severity**: HIGH
- **File**: `admin/includes/ops_workflow.php:216-220`
- **Description**: Dynamic ORDER BY clause construction without proper validation
- **Why it matters**: If user input reaches the $orderColumn variable, SQL injection is possible. Could expose or corrupt data.
- **Suggested fix**: Use a whitelist of allowed column names for ordering.

### 8. Weak Session Configuration
- **Severity**: HIGH
- **File**: `admin/index.php:14-16`
- **Description**: Session name can be customized via potentially untrusted config
- **Why it matters**: Could enable session fixation attacks if config is compromised. Sessions should have secure, hardcoded settings.
- **Suggested fix**: Hardcode session configuration with secure settings; don't trust external config.

## MEDIUM Severity Findings

### 9. Insufficient Financial Input Validation
- **Severity**: MEDIUM
- **File**: `admin/payments.php:10-11`
- **Description**: Financial amounts validated only with float casting
- **Why it matters**: Floating point precision issues can cause rounding errors with money. No validation for negative amounts.
- **Suggested fix**: Store amounts as integer cents, validate for positive values only, use bcmath for calculations.

### 10. Race Condition in Payment Processing
- **Severity**: MEDIUM
- **File**: `admin/payments.php:21-28`
- **Description**: No transaction locking when allocating payments to members
- **Why it matters**: Concurrent requests could double-allocate the same payment. Financial data integrity at risk.
- **Suggested fix**: Wrap payment allocation in database transaction with row-level locking.

### 11. Missing Granular Authorization
- **Severity**: MEDIUM
- **File**: `admin/includes/ops_workflow.php`
- **Description**: Authorization checks only verify admin status, not specific permissions
- **Why it matters**: All admins can perform all operations regardless of intended role. Violates principle of least privilege.
- **Suggested fix**: Implement role-based access control with granular permissions.

### 12. Exposed Sensitive Endpoints
- **Severity**: MEDIUM
- **File**: `_app/api/index.php`
- **Description**: Some API endpoints lack proper authentication checks
- **Why it matters**: Sensitive operations might be accessible to unauthorized users. Data leakage risk.
- **Suggested fix**: Audit all endpoints and ensure consistent authentication/authorization.

## LOW Severity Findings

### 13. Hardcoded Admin Username
- **Severity**: LOW
- **File**: `admin/index.php:126`
- **Description**: Admin email hardcoded as default value in login form
- **Why it matters**: Reveals a valid admin username to potential attackers. Reduces security through obscurity.
- **Suggested fix**: Remove the default value from the login form.

### 14. Inconsistent Type Handling
- **Severity**: LOW
- **File**: Multiple files
- **Description**: Mixed use of strict and loose type comparisons
- **Why it matters**: Could lead to unexpected behavior, especially with "0" vs 0 vs false comparisons.
- **Suggested fix**: Enable strict_types and use consistent type handling.

## Bug Findings

### 15. Undefined Variable
- **Severity**: BUG
- **File**: `_app/api/routes/vault.php:340`
- **Description**: Variable name mismatch - `$investmentTokens` used but `$investTokens` defined
- **Why it matters**: Causes PHP notice/warning and incorrect data processing. Investment calculations may be wrong.
- **Suggested fix**: Use consistent variable naming throughout the function.

### 16. Missing Return Statements
- **Severity**: BUG
- **File**: Multiple functions across codebase
- **Description**: Some functions lack explicit return statements
- **Why it matters**: Functions implicitly return null, which may not match expected behavior. Can cause type errors.
- **Suggested fix**: Add explicit return statements to all functions.

### 17. Date Handling Inconsistencies
- **Severity**: BUG
- **File**: Various admin files
- **Description**: Mix of timezone handling approaches, some using server time, others UTC
- **Why it matters**: Can cause incorrect timestamps, especially for international members. Audit trails may be inaccurate.
- **Suggested fix**: Standardize on UTC for storage, convert for display only.

## Recommendations

### Immediate Actions Required
1. **Delete dangerous files**: Remove `api/dbtest.php` and `resend_admin.php` from all environments
2. **Disable error display**: Set `display_errors = 0` in all production files
3. **Fix SQL injection**: Patch the table name concatenation vulnerability in `ops_workflow.php`

### Short-term Improvements
1. **Implement CSRF protection**: Add tokens to all forms and validate on submission
2. **Add input validation**: Properly validate all financial inputs using integer cents
3. **Fix authorization**: Implement role-based permissions instead of blanket admin access
4. **Add rate limiting**: Protect authentication endpoints from brute force

### Long-term Enhancements
1. **Use prepared statements**: Convert all SQL queries to use parameter binding
2. **Implement proper logging**: Replace error display with structured logging
3. **Add database transactions**: Wrap financial operations in transactions with proper locking
4. **Security headers**: Implement CSP, X-Frame-Options, and other security headers
5. **Regular audits**: Schedule quarterly security reviews

## Summary
The audit identified 3 CRITICAL, 5 HIGH, 4 MEDIUM, and 2 LOW severity security issues, plus 3 bugs. The most urgent concerns are the exposed admin tools and SQL injection vulnerability. Financial processing also needs attention to prevent data integrity issues.

Priority should be given to removing dangerous files, fixing injection vulnerabilities, and implementing proper authentication/authorization across all admin endpoints.