# Auditoría de Seguridad y Arquitectura — `kleeplanner`

> **Fecha:** 23 de abril de 2026
> **Auditor (rol):** Arquitecto Senior / Especialista OWASP
> **Alcance:** revisión estática del código fuente del repositorio `kleeplanner` (framework PHP propio "Klee", frontend Metronic + JS, MySQL, autenticación local + SSO Microsoft Entra ID).
> **Tipo:** auditoría de caja blanca (white-box), sin pentesting dinámico.

---

## 0. Contexto técnico confirmado

| Item | Valor |
|---|---|
| Servidor web | Apache (HTTP) |
| PHP | 8.3.6 NTS |
| BD | MySQL (`klee_planner`) |
| Frontend | Metronic 8 + JS plano + jQuery |
| Auth | Local (usuario+contraseña MD5) + SSO Microsoft Entra (OAuth2 manual con cURL) |
| `BASE_FILES` | `BASE_PATH.'/files/'` (carpeta dentro del proyecto, expuesta vía web) |
| WAF / CDN | **No existe** (servidor expuesto directo a Internet) |
| Tenant Microsoft / política de cuentas | **No definido / no se sabe** (alto riesgo) |
| Integraciones externas | Transferencia por **SFTP** (sin detalle de rotación de claves) |
| PII en reposo | **No cifrada** (cédulas, correos, contratos en texto plano) |
| Backups | Servidor secundario, **semanales (viernes)**. RPO = 7 días |
| Deploy | `git pull` manual sobre `master`. Sin CI/CD, sin migraciones controladas |
| Flujo Git | Rama personal → `developer` (validación) → `master`, **sin Pull Request, sin code review formal** |

> Estos puntos por sí solos elevan el perfil de riesgo del sistema a **CRÍTICO**: backups semanales con datos no cifrados + sin WAF + secretos en repo + deploy por `git pull` significan que cualquier compromiso del repositorio o del servidor implica fuga total de PII de talento humano.

---

## 1. Resumen ejecutivo

`kleeplanner` está construido sobre un mini-framework MVC propio ("Klee") creado antes de la era de los frameworks modernos. La aplicación **funciona**, pero su capa de seguridad arrastra deuda estructural que combina **vulnerabilidades críticas explotables sin esfuerzo** con **decisiones de diseño que impiden remediar de forma puntual** (hay que rediseñar capas completas: BD, sesión, subida de archivos, permisos).

### Riesgos de mayor impacto (Top 13)

| # | Hallazgo | Severidad | Impacto principal |
|---|---|---|---|
| **C1** | Backdoor de autenticación: contraseña maestra `123` válida vía `MysqlPDO::validateUser` + `Config::getPassAccess()` | 🔴 Crítica | Toma de control total de cualquier cuenta |
| **C2** | Contraseñas almacenadas con **MD5 sin sal** (`PassHelper`) y comparadas con `==` | 🔴 Crítica | Cracking masivo en minutos si fuga la BD |
| **C3** | **Secretos hardcodeados** y comprometidos en repo: BD, OAuth Microsoft (`client_secret`), API key OpenAI, credenciales SMTP | 🔴 Crítica | Impersonación, abuso de billing, acceso a BD |
| **C4** | **SQL Injection sistémica**: `addslashes` + concatenación, `queryAllSql` con `$_GET`, `criteriaToSql` interpola operadores y valores | 🔴 Crítica | Exfiltración total de datos, modificación, RCE potencial |
| **C5** | **Sin protección CSRF** en ningún endpoint state-changing | 🔴 Crítica | Acciones forzadas sobre usuarios autenticados |
| **C6** | OAuth Microsoft mal implementado: sin `state`, sin validación de `id_token`, `client_secret` en código, sin verificación de tenant | 🔴 Crítica | Bypass de SSO / login forgery |
| **C7** | Subida de archivos con validación por MIME del cliente; almacenamiento dentro del webroot (`/files/`) sin `.htaccess` que deshabilite ejecución | 🔴 Crítica | RCE vía upload de `shell.php` o doble extensión |
| **C8** | Endpoints de API con `AccessControl='*'` que devuelven PII (cédula, correo, cargo, jefe) sin rate-limit ni paginación obligatoria | 🟠 Alta | Fuga masiva de datos personales |
| **C9** | XSS reflejado y almacenado: `echo $e`, `echo "<script>alert('$correo')..."`, vistas que imprimen modelo sin escape | 🟠 Alta | Secuestro de sesión |
| **C10** | **IDOR ubicuo**: `deleteById($_GET['Id'])`, `loadById($_POST['Id'])` sin validar propietario/alcance | 🟠 Alta | Borrado y modificación de registros ajenos |
| **C11** | Sesiones sin `HttpOnly`/`Secure`/`SameSite`, sin `session_regenerate_id` al login → **fijación de sesión** | 🟠 Alta | Hijack de sesión |
| **C12** | `public/test_api_diag.php` accesible públicamente; cada visita consume la API key de OpenAI | 🟠 Alta | Abuso de billing / DoS financiero |
| **C13** | Login: enumeración de usuarios + sin rate-limit + sin lockout + sin CAPTCHA + mensajes diferenciados | 🟡 Media | Fuerza bruta y enumeración |

### Conclusión ejecutiva
- **No es seguro mantener el sistema en producción tal como está.**
- Las correcciones C1, C2, C3, C5, C7, C12 deben aplicarse **inmediatamente** (horas, no semanas).
- C4, C6, C10 requieren refactor de capas (capa DB, capa Auth, capa de permisos row-level): plan a 2–4 sprints.
- A mediano plazo (6–12 meses) recomiendo migrar a **Laravel 11 / Symfony 7**, modulando por dominio (Inducciones, Talento, Reportes). El costo de mantener "Klee" supera el costo del migrate.

---

## 2. Modelo de amenazas (resumen STRIDE)

| Activo | Amenaza dominante | Vector | Impacto |
|---|---|---|---|
| BD `klee_planner` (PII) | **Tampering / Information disclosure** | SQLi en cualquier `$_GET` que llegue al QueryBuilder; backups sin cifrar en otro servidor | Crítico (Habeas Data, Ley 1581 Colombia) |
| Sesiones de usuario | **Spoofing / Elevation of privilege** | Backdoor `123`, fijación de sesión, sin CSRF | Crítico |
| `/files/` (CV, contratos, soportes inducción) | **Tampering / RCE** | Upload PHP en webroot; path traversal en `CrearCarpetas` | Crítico |
| Token OAuth Microsoft | **Spoofing** | Sin validación de `state`/`id_token`/tenant | Alto |
| Servidor producción | **DoS / RCE** | Sin WAF, sin rate-limit, scripts diagnóstico expuestos | Alto |
| Repositorio Git | **Information disclosure** | Secretos en `Config.php`, `LoginController.php`, `ConfigEnv.php` | Crítico |
| Pipeline deploy (`git pull`) | **Tampering** | Sin firmas, sin tags, sin revisión PR; commit malicioso entra en producción al siguiente pull | Alto |

---

## 3. Vulnerabilidades priorizadas (con evidencia)

### 🔴 C1 — Backdoor maestro de contraseña

**Archivo:** `app/config/Config.php`
```php
public static function getPassAccess(){ return PassHelper::encode('123'); }
public static function getUserAccess(){ return 1; }
```
**Archivo:** `core/db/MysqlPDO.php`
```php
public function validateUser($user, $pass, $table = null) {
    ...
    return PassHelper::verify($pass, $usuario['Contrasena'])
        || PassHelper::verify($pass, Controller::getPassAccess());
}
```
Cualquier código que invoque `MysqlPDO::validateUser` permite ingresar con `123`. Aunque hoy `UsuariosModel::validateUser` usa otra ruta, el método sigue accesible y `getUserAccess()` retorna `1` (id típico de superadmin) usado en `Controller::validateAccess` como bypass de permisos.

**Solución (inmediata):**
- Eliminar `Config::getPassAccess()` y `Config::getUserAccess()`.
- Eliminar `MysqlPDO::validateUser`.
- `grep -r getPassAccess|getUserAccess` y depurar usos.

---

### 🔴 C2 — Contraseñas en MD5

**Archivo:** `core/helpers/PassHelper.php`
```php
class PassHelper {
    public static function encode($pass){ return md5($pass); }
    public static function verify($pass,$hash){ return md5($pass)==$hash; }
}
```
- Sin sal → ataque por rainbow tables.
- Sin stretching → GPU rompe millones por segundo.
- `==` no es comparación tiempo-constante (timing attack).

**Solución:**
```php
class PassHelper {
    public static function encode(string $pass): string {
        return password_hash($pass, PASSWORD_ARGON2ID);
    }
    public static function verify(string $pass, string $hash): bool {
        // Compatibilidad con MD5 legacy mientras se migra
        if (strlen($hash) === 32 && ctype_xdigit($hash)) {
            return hash_equals($hash, md5($pass));
        }
        return password_verify($pass, $hash);
    }
    public static function needsRehash(string $hash): bool {
        return password_needs_rehash($hash, PASSWORD_ARGON2ID);
    }
}
```
En `UsuariosModel::validateUser`, tras login OK: si `needsRehash`, re-hashear y `UPDATE`.

---

### 🔴 C3 — Secretos hardcodeados

**Evidencia:**
- `app/config/ConfigEnv.php`: `'password' => 'Klee321@'` y credenciales SMTP Mailtrap.
- `app/controllers/LoginController.php`:
  ```php
  $application_id = '7e654f6d-b616-48f3-bb9a-1986c0e0aac3';
  $application_secret='64B8Q~UaUm-kJ_FEGsigMBFNZHWytcZU9SbrjcdI';
  ```
- `app/config/Config.php`:
  ```php
  public static $AI_API_KEY = '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';
  ```
- `public/test_api_diag.php` reutiliza la clave y la expone vía cualquier visita HTTP.

**Acción inmediata (mismo día):**
1. **Asumir TODAS las claves comprometidas** y rotarlas en sus respectivos paneles:
   - Microsoft Entra: revocar `application_secret`, generar nuevo.
   - OpenAI: revocar `sk-proj-...`, generar nuevo, configurar **límite de gasto** (soft+hard cap).
   - MySQL: cambiar password del usuario `klee_user`, restringir a `localhost` o IP específica.
   - SMTP / Mailtrap: rotar.
2. Mover a `.env` fuera del webroot:
   ```
   composer require vlucas/phpdotenv
   ```
   ```php
   // public/index.php
   $dotenv = Dotenv\Dotenv::createImmutable(BASE_PATH);
   $dotenv->safeLoad();
   ```
   Acceder por `getenv('DB_PASSWORD')`.
3. Añadir `.env` y `.env.*` al `.gitignore`.
4. **Purgar el historial** del repo (las claves ya están en commits anteriores):
   ```bash
   git filter-repo --invert-paths --path app/config/ConfigEnv.php
   ```
   o usar `bfg-repo-cleaner`.
5. Instalar pre-commit `gitleaks` para evitar reincidencia.

---

### 🔴 C4 — SQL Injection sistémica

#### 4.a Query builder propenso
**Archivo:** `core/db/MysqlPDO.php`
```php
public function insert($table, $fields = array(), $values = array()) {
    $values = array_map("addslashes", array_values($values));
    $values = array_map(function($n){ ... return "'".$n."'"; }, $values);
    ...
    $sql = "INSERT INTO ".$table." (".$fields.") VALUES (".$values." );";
    return $this->sql($sql);
}
```
- `addslashes` **no es defensa contra SQLi**; bypass conocidos con `SET NAMES gbk` u otros multi-byte.
- Identificadores (`$table`, `$fields`) se concatenan crudo.

#### 4.b `criteriaToSql` permite valores raw
```php
'value' => "Ceco LIKE '".'%'."V' "
```
Este patrón se usa decenas de veces (`AjaxController`) para meter SQL crudo bajo el campo `value`. Si `$_GET` aterriza ahí, hay inyección directa.

#### 4.c Concatenación con `$_GET` directo
`app/controllers/AjaxController.php`:
```php
$filtros[] = "NoIdentificacion LIKE '".$_GET['docente']."%' OR CONCAT(Nombre,' ',Apellido) LIKE '".$_GET['docente']."%'";
```
`app/controllers/ApiController.php`:
```php
'value' => "(".$_GET['estado'].")"   // operator IN, separatorValues ''
```
`app/controllers/ArchivosController.php`:
```php
Model::queryAllSql("UPDATE _programaciones SET INICIO_NRC ='".$date->format('Y-m-d')."' WHERE INICIO_NRC='".$consulta['INICIO_NRC']."'");
Model::queryAllSql("SELECT * FROM _programaciones WHERE PERIODO_CODIGO='".$consulta['PERIODO_CODIGO']."' AND NRC='".$consulta['NRC']."' AND DOCUMEN_PROFESOR IN(".$id_docente.")");
```
`ReportesController.php` construye SQL crudo en bucle (líneas 5684+, 5917+, 6603+, 6865+).

#### 4.d `ORDER BY` injection
`criteriaToSql` interpola `ORDER_BY['COLUMN']`. Cualquier endpoint que reciba el orden desde el cliente es injectable (no se puede parametrizar `ORDER BY` con prepared statements).

**Solución (capa nueva `core/db/SafePDO.php`):**
```php
class SafePDO {
    private PDO $pdo;
    public function __construct(PDO $pdo) { $this->pdo = $pdo; }

    public function selectOne(string $sql, array $params = []): ?array {
        $st = $this->pdo->prepare($sql);
        $st->execute($params);
        $row = $st->fetch(PDO::FETCH_ASSOC);
        return $row ?: null;
    }
    public function selectAll(string $sql, array $params = []): array {
        $st = $this->pdo->prepare($sql);
        $st->execute($params);
        return $st->fetchAll(PDO::FETCH_ASSOC);
    }
    public function exec(string $sql, array $params = []): int {
        $st = $this->pdo->prepare($sql);
        $st->execute($params);
        return $st->rowCount();
    }
}
```
- Validar identificadores (tabla/columna) contra `Model::getOptionsAttributes()` antes de inyectar.
- Whitelistear `operator` y `direction`.
- Para `IN (?,?,?)`, expandir el array y pasar parámetros.
- En `Model::queryAllSql`, exigir parámetros bound; deprecar `queryAllSql($sql)` raw.

> Migración recomendada: **Doctrine DBAL** (capa fina sobre PDO con QueryBuilder seguro). Permite reemplazar gradualmente sin rehacer modelos.

---

### 🔴 C5 — CSRF ausente

Ningún form/AJAX emite ni valida token. Logout, cambios de estado, deletes, masivos: todos vulnerables.

**Solución (núcleo nuevo `core/Csrf.php`):**
```php
class Csrf {
    public static function token(): string {
        if (empty($_SESSION['_csrf'])) $_SESSION['_csrf'] = bin2hex(random_bytes(32));
        return $_SESSION['_csrf'];
    }
    public static function check(): void {
        $t = $_POST['_csrf'] ?? $_SERVER['HTTP_X_CSRF_TOKEN'] ?? '';
        if (!hash_equals($_SESSION['_csrf'] ?? '', $t)) {
            http_response_code(419); exit('CSRF token mismatch');
        }
    }
}
```
Inyectar en `Controller::process()`:
```php
if (!in_array($_SERVER['REQUEST_METHOD'], ['GET','HEAD','OPTIONS'], true)) {
    Csrf::check();
}
```
En layout Metronic:
```html
<meta name="csrf-token" content="<?= htmlspecialchars(Csrf::token()) ?>">
```
En `core/ajax.js`: añadir header global `X-CSRF-Token` a todo `fetch`/`$.ajax`.

---

### 🔴 C6 — OAuth Microsoft (`LoginController::validatedAction`)

Problemas:
1. No envía `state` aleatorio → CSRF de OAuth posible.
2. No valida el `id_token` (firma JWKS, `iss`, `aud`, `tid`, `exp`, `nonce`).
3. Confía en `userPrincipalName` retornado por Graph; cualquier cuenta Microsoft con UPN coincidente entra (incluyendo invitados externos del tenant).
4. `client_secret` en código.
5. No invalida sesión si `validate1User` falla; queda en limbo.

**Solución:**
```bash
composer require thenetworg/oauth2-azure firebase/php-jwt
```
- Persistir `state` y `nonce` en `$_SESSION` antes del redirect.
- Validar `id_token` con JWKS de `https://login.microsoftonline.com/<tenant>/discovery/v2.0/keys`.
- Validar `tid == TENANT_AUTORIZADO` (definir uno) y `aud == client_id`.
- Configurar la app en Entra como **single-tenant** o limitar dominios de email vía claim.
- Redirect URI desde `.env`.

---

### 🔴 C7 — Subida de archivos

Patrón generalizado (`InduccionesController`, `Plantillas`, `Cumpleanos`, `RegistroParticipacionCOIL`, `HistoricoReportePlano`):
```php
$fileType = $_FILES['Archivo']['type']; // CONTROLADO POR EL CLIENTE
if (!in_array($fileType, ['text/csv','application/vnd.ms-excel'])) { ... }
$destino = BASE_PATH."files/csv/".date("YmdHis").uniqid().'.csv';
move_uploaded_file($fileTmp, $destino);
```
Adicional:
- En varios sitios usan `copy()` en vez de `move_uploaded_file()` → omite verificación de upload legítimo.
- `BASE_PATH/files/` es navegable bajo Apache (no hay `.htaccess` que deniegue ejecución de PHP).
- `ArchivosController::exportPDFAction` recibe `$_POST['contenido']` HTML libre y lo manda a mPDF → SSRF/LFI vía `<img src="file:///...">` y descarga remota de imágenes.

**Solución (helper único, fuera del webroot):**

1. Mover `files/` a `BASE_PATH/../storage/` (fuera de `public/`).
2. Helper:
```php
final class UploadHelper {
    /** @param string[] $allowedMime */
    public static function store(array $file, array $allowedMime, string $folder, int $maxBytes = 10_485_760): string {
        if (!is_uploaded_file($file['tmp_name'] ?? '')) throw new RuntimeException('not_uploaded');
        if (($file['error'] ?? UPLOAD_ERR_NO_FILE) !== UPLOAD_ERR_OK) throw new RuntimeException('upload_err');
        if ($file['size'] > $maxBytes) throw new RuntimeException('too_big');

        $mime = (new finfo(FILEINFO_MIME_TYPE))->file($file['tmp_name']);
        if (!in_array($mime, $allowedMime, true)) throw new RuntimeException('mime_'.$mime);

        $ext = match ($mime) {
            'application/pdf' => 'pdf',
            'image/png' => 'png',
            'image/jpeg' => 'jpg',
            'text/csv','text/plain' => 'csv',
            'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' => 'xlsx',
            default => throw new RuntimeException('ext_'.$mime),
        };
        if (!is_dir($folder)) mkdir($folder, 0750, true);
        $name = bin2hex(random_bytes(16)).'.'.$ext;
        $dest = rtrim($folder, '/').'/'.$name;
        if (!move_uploaded_file($file['tmp_name'], $dest)) throw new RuntimeException('move_failed');
        return $name;
    }
}
```
3. Servir descargas por controller con auth + `Content-Disposition: attachment`.
4. Defensa en profundidad — si no es posible mover `files/` ya, añadir `.htaccess`:
```apache
# /files/.htaccess
<FilesMatch "\.(php|phtml|phar|phps|pl|py|cgi|sh|asp|aspx|htaccess)$">
    Require all denied
</FilesMatch>
Options -ExecCGI -Indexes -FollowSymLinks
RemoveHandler .php .phtml
AddType text/plain .php .phtml .phar
SetHandler None
```
5. mPDF endurecido:
```php
$mpdf = new \Mpdf\Mpdf([
    'allow_html_fragment_links' => false,
    'curlAllowUnsafeSslRequests' => false,
]);
$mpdf->showImageErrors = false;
$mpdf->whitelistStreamWrappers = []; // bloquea file://, http://, etc.
// Sanitizar el HTML antes:
$clean = (new \HTMLPurifier())->purify($_POST['contenido']);
```

---

### 🟠 C8 — APIs públicas con PII (`ApiController`)

```php
'plantaActiva' => '*',
'programacionHorarios' => '*',
'labor' => '*',
```
- Auth por Bearer pero token comparado con `password_verify` contra `ConfiguracionesModel::getValor('ApiToken')`. Si el valor almacenado **no es un hash bcrypt**, `password_verify` retorna `false` siempre (fail-closed bien) — pero si alguien ha guardado el token en plano para "facilidad", el endpoint quedaría inaccesible o, peor, accesible si alguien adapta el código.
- Sin rate-limit, sin paginación obligatoria, sin escopes, sin audit log. Devuelve `'*'` (todas las columnas).

**Solución:**
- `AccessControl => '@'` con auth por sesión, **o** middleware API con JWT firmado (HS256/RS256), `iss`, `aud`, `exp`, `scope`.
- Rate-limit por IP+token (Redis + sliding window).
- Devolver sólo columnas necesarias y obligar `LIMIT` máx (p. ej. 500 por request).
- Log estructurado por petición.

---

### 🟠 C9 — XSS

- `InduccionesController`:
  ```php
  } catch (PDOException $e) { echo $e; }
  echo "<script>alert('Error: ...'); window.history.back();</script>";
  ```
- `Controller::loadFiltersSesion()` concatena `$item['Nombre']` en HTML sin escape.
- Vistas Metronic en `app/views/**/list.php` muestran datos del modelo con `echo $row['Campo']`.

**Solución:**
- Helper global:
  ```php
  function e(?string $s): string { return htmlspecialchars((string)$s, ENT_QUOTES|ENT_SUBSTITUTE, 'UTF-8'); }
  ```
- Auditar y reemplazar todos los `echo $variable` por `<?= e($variable) ?>`.
- Eliminar `echo $exception`. Logger interno (Monolog) + página de error genérica.
- Header `Content-Security-Policy: default-src 'self'; script-src 'self' 'nonce-<RND>'; style-src 'self' 'unsafe-inline'; object-src 'none'; base-uri 'self'`.

---

### 🟠 C10 — IDOR (Insecure Direct Object Reference)

Patrón típico:
```php
$this->Model->deleteById($_GET['Id']);
$this->Model->loadById($_POST[get_class($this->Model)]['Id']);
```
El sistema de permisos `PermisosModel::hasAccess($controller,$action)` es por módulo+acción, **no por recurso**. Un usuario con rol "consulta" en `Inducciones` puede modificar/borrar cualquier inducción si conoce el `Id`.

**Solución:**
- Definir alcance por sesión (`SedeId`, `EscuelaId`, `UserId`).
- Cada `getById/loadById/deleteById` debe enriquecerse con criterio del usuario:
  ```php
  $row = InduccionesModel::getByIdScoped($id, ['Sede' => $_SESSION[...]['User']['Sede']]);
  if (!$row) throw new ForbiddenException();
  ```
- Auditoría obligatoria en `LogAccionesModel` con `user_id`, `recurso`, `accion`.

---

### 🟠 C11 — Sesiones inseguras

`public/index.php` no configura cookies. PHP por defecto: sin `Secure`, sin `HttpOnly`, sin `SameSite`, sin `use_strict_mode`. Tampoco se llama `session_regenerate_id(true)` al login → fijación de sesión.

**Solución (añadir antes de cualquier `session_start()`):**
```php
ini_set('session.use_strict_mode', '1');
ini_set('session.use_only_cookies', '1');
ini_set('session.cookie_httponly', '1');
ini_set('session.cookie_secure', '1');          // requiere HTTPS forzado
ini_set('session.cookie_samesite', 'Lax');      // 'Strict' si no hay redirects desde MS
ini_set('session.gc_maxlifetime', '1800');      // 30 min inactividad
session_set_cookie_params([
    'lifetime' => 0,
    'path' => '/',
    'secure' => true,
    'httponly' => true,
    'samesite' => 'Lax',
]);
session_start();
```
En `UsuariosModel::validateUser` tras login OK: `session_regenerate_id(true);`.
En `LoginController::logoutAction`:
```php
$_SESSION = [];
session_destroy();
setcookie(session_name(), '', time()-3600, '/');
```

---

### 🟠 C12 — Script de diagnóstico expuesto

`public/test_api_diag.php`:
```php
$key = Config::$AI_API_KEY;
... CURLOPT_SSL_VERIFYPEER => false ...
```
Cualquier visita ejecuta una llamada a OpenAI cobrada a la cuenta. Además desactiva validación TLS.

**Acción inmediata:** **borrar el archivo**. Si se necesita diagnóstico, mover fuera de `public/` y proteger con auth admin.

---

### 🟡 C13 — Login sin protección anti-fuerza-bruta

`LoginController::validateAction`:
- Mensajes diferentes (`no_user`, `pass_wrong`, `user_inactive`, `unauthorized_ip`) → enumeración.
- Sin throttling.
- Sin CAPTCHA.
- Sin lockout.

**Solución:**
- Mensaje único: "Credenciales inválidas".
- Tabla `login_attempts (Id, Usuario, Ip, FechaHora, Resultado)`.
- Bloquear 15 min tras 5 fallos por usuario o por IP.
- CAPTCHA (hCaptcha/Turnstile) tras 3 fallos.
- Forzar 2FA o SSO Microsoft (deshabilitar login local cuando sea posible).

---

## 4. Hallazgos adicionales (severidad media/baja)

| Id | Hallazgo | Archivo | Severidad |
|---|---|---|---|
| M1 | `Formvalidate` usa `$_REQUEST` (mezcla GET/POST/COOKIE → confusión + posible bypass) | `core/Formvalidate.php` | Media |
| M2 | `mPDF::Output(.., $_GET['Destination'])` permite destino arbitrario (`F` escribe al disco) | `ArchivosController.php` | Media |
| M3 | `ArchivosController::generalAction` imprime `dbname` al cliente (fuga) | idem | Baja |
| M4 | `classesGeneral=['Public','Home','Ajax','Log','Login']` deja **AJAX abierto** sin permisos | `Config.php` | Alta |
| M5 | `Controller::CrearCarpetas($Ruta)` hace `mkdir` con ruta concatenada, potencial path traversal | `Controller.php` | Media |
| M6 | `ConfigEnv::DB_CONNECTIONS` con `host=localhost` y user con (probablemente) `ALL PRIVILEGES`; `ArchivosController` ejecuta `ALTER TABLE` y `AUTO_INCREMENT=1` desde HTTP | varios | Alta |
| M7 | `Config::$debug` activa `console.log` de SQL en HTML — XSS auto-infligido si se enciende en producción | `Model.php`, `MysqlPDO.php` | Media |
| M8 | `LogsConsole::setLog('DB', $sql)` en HTML (debug ON) → fuga de estructura BD | `LogsConsole.php` | Baja |
| M9 | `public/check_session.php` responde `1/0` sin CORS check → oráculo de sesión cross-origin si se relaja CORS | `public/check_session.php` | Baja |
| M10 | `ApiToken` se guarda en `configuraciones`; si se editó manualmente como texto plano, `password_verify` siempre falla. No hay procedimiento de rotación documentado | `ApiController.php` | Media |
| M11 | Sin `declare(strict_types=1)`, sin namespaces, sin PSR-4: autoload custom basado en globals (riesgo de colisión y dificultad de testing) | global | Media |
| M12 | Errores expuestos: `display_errors` no se fuerza `Off`, posible fuga de paths/stacktrace | global | Media |
| M13 | `header('Content-Type: text/html; charset=utf-8')` en `Controller::__construct` se aplica a TODOS, incluso APIs JSON; se sobreescribe luego pero queda fragil | `Controller.php` | Baja |
| M14 | Backups semanales **sin cifrar** y en otro servidor: si ese servidor se compromete, fuga total. RPO 7 días excesivo para HR. | infra | Alta |
| M15 | Deploy por `git pull` sin verificación de firma/tag, sin migraciones controladas: rollback manual y arriesgado | infra | Alta |
| M16 | Sin PR / sin code review: backdoors o vulnerabilidades pueden colarse a `master` sin segundo par de ojos | proceso | Alta |
| M17 | Ausencia de WAF/CDN: ataques automatizados (escáneres) golpean el servidor directo. Sin protección DDoS L7 | infra | Alta |

---

## 5. Análisis por módulo

### 5.1 Login / Autenticación
- **Función:** login local + SSO Microsoft.
- **Riesgos clave:** C1, C2, C3, C6, C11, C13.
- **Cómo se rompe:** (a) login con `123` si `MysqlPDO::validateUser` aún se invoca; (b) phishing OAuth sin `state`; (c) fuga de BD → MD5 crackeable → reuso en otros sistemas.

### 5.2 Inducciones (`InduccionesController`)
- **Función:** CRUD inducciones, envío de correos masivos, cambio de estado `asistio`, carga CSV de cédulas, subida de soportes.
- **Riesgos:** IDOR (`deleteById($_GET['Id'])`), CSRF en `asistioMasivoAction`, validación de adjuntos por MIME del cliente, `bodyMail` recibe HTML libre desde `$_POST` y se envía como correo (phishing interno con marca corporativa), `$_POST['Estado']` sin whitelist.
- **Explotación plausible:** un usuario operador (con CSRF de un atacante) marca masivamente "asistido" a empleados que no asistieron; o sube un archivo `.csv` doble extensión → ejecutado si Apache no bloquea PHP en `/files/`.

### 5.3 Carga de adjuntos (`Archivos`, `ArchivosPerfiles`, `Plantillas`, `Cumpleanos`, `RegistroParticipacionCOIL`, `HistoricoReportePlano`)
- **Riesgos:** C7. Adicional: rutas con `date("YmdHis").uniqid()` no son criptográficamente fuertes pero suficientes contra colisión; el problema real es la ejecución y el contenido.
- **Explotación:** subir `pwned.php` con MIME `text/csv`, navegar a `/files/csv/<nombre>.php`.

### 5.4 Validación por cédula (`AjaxController::getIdBlockedAction`, `getDataSolicitudAction`, `ApiController::plantaActiva`)
- **Riesgos:** SQLi via `criteriaToSql` si se manipula el formato del request; fuga de PII (devuelve `array('*')`); enumeración de cédulas válidas (oráculo).
- **Explotación:** rebote masivo a `?cedula=<n>` para enumerar empleados activos/bloqueados.

### 5.5 Reportes (`ReportesController` ~6900 líneas)
- **Función:** reportes de docentes, posgrado, pregrado, contratados, eventos, etc.
- **Riesgos:** SQLi por construcción de SQL crudo dentro de bucles; carga total en memoria sin streaming → OOM con datasets grandes; sin paginación.
- **Mantenibilidad:** controlador inmantenible. Refactor obligatorio: separar `services/Reportes/*` por dominio, usar generadores PHP (`yield`) para streaming a Excel/CSV.

### 5.6 Cambio de estado
- **Riesgos:** C5 (CSRF), C10 (IDOR), ausencia de auditoría firmada (HMAC). `LogAccionesModel` puede ser borrado vía SQLi.

### 5.7 API (`ApiController`)
- **Riesgos:** C4, C8. Sin versionado, sin documentación, sin CORS explícito.

### 5.8 Capa BD (`core/db/MysqlPDO.php`)
- **Riesgos:** C4 estructural. `ORDER BY` injection inevitable con la API actual.

### 5.9 Permisos (`PermisosModel`, `Controller::validateAccess`)
- **Riesgos:** granularidad por módulo+acción, sin row-level. `classesGeneral` da barra libre.

---

## 6. Recomendaciones técnicas concretas

### 6.1 Capa de datos
- Introducir `SafePDO` (sección C4) y migrar consultas progresivamente.
- Whitelist de columnas/tablas/operadores en `criteriaToSql` (basada en metadata del modelo).
- Para cargas masivas: mover `ALTER TABLE` y `AUTO_INCREMENT=1` a scripts de migración (`/db/migrations/*.sql`) ejecutados por CLI con un usuario DBA distinto, **nunca desde HTTP**.
- Crear **dos usuarios MySQL**:
  - `klee_app` (SELECT, INSERT, UPDATE, DELETE) para la app.
  - `klee_migrate` (DDL) sólo para migraciones desde CLI.

### 6.2 Autenticación / Sesión
- Borrar backdoor C1.
- Migrar contraseñas a Argon2id (C2).
- Sesiones endurecidas (C11).
- Lockout + CAPTCHA + mensaje único (C13).
- **Forzar HTTPS** y HSTS (Apache):
  ```apache
  <VirtualHost *:80>
      ServerName polimero.poligran.edu.co
      Redirect permanent / https://polimero.poligran.edu.co/
  </VirtualHost>
  Header always set Strict-Transport-Security "max-age=31536000; includeSubDomains"
  ```

### 6.3 OAuth Microsoft
- Reemplazar implementación cURL por `thenetworg/oauth2-azure`.
- Validar `id_token` (JWKS).
- Configurar app como single-tenant (o restringir `tid`).
- Considerar **deshabilitar login local** para empleados — sólo SSO + MFA (lo gestiona Entra).

### 6.4 Subida de archivos
- Helper único `UploadHelper` (C7).
- Mover `files/` fuera de `public/`.
- `.htaccess` defensivo si no se mueve.
- Antivirus en pipeline:
  ```bash
  apt install clamav
  ```
  Llamar `clamscan --no-summary <ruta>` antes de aceptar el archivo.

### 6.5 CSRF / Headers
- Middleware `Csrf::check()` (sección C5).
- Cabeceras globales en `.htaccess`:
  ```apache
  Header always set X-Frame-Options "DENY"
  Header always set X-Content-Type-Options "nosniff"
  Header always set Referrer-Policy "strict-origin-when-cross-origin"
  Header always set Permissions-Policy "geolocation=(), microphone=(), camera=()"
  Header always set Content-Security-Policy "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self'; object-src 'none'; base-uri 'self'; frame-ancestors 'none'"
  Header unset X-Powered-By
  ```
  En `php.ini`: `expose_php = Off`, `display_errors = Off`, `log_errors = On`.

### 6.6 Logging y observabilidad
- Centralizar con **Monolog** + rotación + envío a archivo `/var/log/kleeplanner/*.log`.
- Auditoría firmada: `LogAccionesModel` con `Hash = HMAC(secret, prev_hash || row)` (cadena tipo blockchain ligera). Imposible borrar sin romper la cadena.
- Integrar **Sentry** o **GlitchTip** (self-hosted) para errores en tiempo real.

### 6.7 Datos personales (Habeas Data — Ley 1581 Colombia)
- Cifrar columnas sensibles en reposo (cédula, dirección, teléfono, salario, contratos):
  - Opción A: cifrado a nivel app con `sodium_crypto_secretbox` (clave en `.env`, rotación documentada).
  - Opción B: cifrado a nivel BD con MariaDB/MySQL `ENCRYPTION='Y'` o ProxySQL.
- Backups cifrados con GPG/age, llaves separadas.
- Política de retención (ej. ex-empleados → 10 años por norma laboral COL, después purgar).

### 6.8 Infraestructura
- Poner **Cloudflare** (plan gratuito alcanza para WAF básico + bot fight + rate-limit + bloqueo geográfico de IPs no LATAM).
- O ModSecurity + reglas OWASP CRS en Apache:
  ```apache
  IncludeOptional /etc/modsecurity/*.conf
  ```
- Aislar BD en red privada (no `bind 0.0.0.0`).
- Backups **diarios** cifrados, **probar restauración mensual**.
- HTTPS obligatorio con Let's Encrypt + auto-renovación.

### 6.9 Proceso / DevOps
- Implementar **Pull Requests obligatorios** en GitHub/GitLab/Bitbucket: nadie commitea directo a `master`/`developer`.
- Mínimo 1 reviewer + checks automáticos:
  - PHPStan nivel 6+
  - Psalm
  - `gitleaks` (secretos)
  - `composer audit` (CVEs)
  - PHPUnit (al menos smoke tests de auth/upload/SQL)
- Reemplazar `git pull` por pipeline:
  ```
  build → tests → push artifact → deploy con tag firmado
  ```
  Herramienta sugerida: **Deployer** (`deployer.org`) con releases y rollback en 1 comando.

### 6.10 Mediano/largo plazo
- Migración progresiva a **Laravel 11** (Strangler Fig pattern):
  - Mes 1–2: levantar Laravel paralelo con auth + módulo de inducciones.
  - Mes 3–6: migrar reportes y APIs.
  - Mes 6–12: deprecar Klee.
- Beneficios: ORM seguro, CSRF/sesión/policies de fábrica, Horizon (jobs), Telescope (debug), ecosistema activo.

---

## 7. Quick wins (aplicables hoy mismo)

> Tiempo estimado total: 1 día de trabajo concentrado. Reducen ~70% del riesgo crítico.

1. **Borrar** `public/test_api_diag.php`.
2. **Rotar TODAS las claves** (DB, Microsoft `client_secret`, OpenAI, SMTP).
3. Mover secretos a `.env` + `.gitignore` + purgar historial con `git filter-repo`.
4. Eliminar `Config::getPassAccess()`, `Config::getUserAccess()` y `MysqlPDO::validateUser()` (backdoor).
5. Configurar sesión segura en `public/index.php` (snippet C11).
6. `session_regenerate_id(true)` al final de `UsuariosModel::validateUser`.
7. `LoginController::validateAction`: mensaje único "Credenciales inválidas".
8. Forzar `Config::$debug = false` en producción; eliminar `echo $e` y `console.log` de SQL.
9. `.htaccess` en `/files/` deshabilitando ejecución (snippet C7.4).
10. `.htaccess` raíz con headers de seguridad (sección 6.5).
11. Cambiar `AccessControl '*'` → `'@'` en `ApiController` y `AjaxController` donde sea posible.
12. `grep -r "CURLOPT_SSL_VERIFYPEER => false"` y eliminar.
13. Forzar HTTPS + HSTS (Apache).
14. Borrar `Config::$AI_API_KEY` del repo (rotar antes).
15. Configurar **límite de gasto** en panel OpenAI (hard cap mensual).
16. Activar **Cloudflare** delante del dominio (15 min de configuración).
17. Cambiar password del usuario MySQL `klee_user` y restringir su host.

---

## 8. Mejoras a mediano y largo plazo

### Mediano (1–3 meses)
- Implementar `SafePDO` y migrar 100% de las queries crudas (`queryAllSql` con interpolación) y endpoints AJAX/API críticos.
- Implementar CSRF global + helper `e()` y barrer vistas.
- Implementar OAuth Microsoft con librería + JWKS.
- Mover `files/` fuera del webroot + helper de upload con `finfo` + ClamAV.
- Lockout/CAPTCHA en login.
- Cifrar columnas PII con `sodium_crypto_secretbox`.
- Backups diarios cifrados, prueba mensual de restauración.
- Implementar PRs obligatorios + CI con PHPStan, gitleaks, composer audit.
- Reemplazar `git pull` por Deployer.

### Largo (3–12 meses)
- Migrar a **Laravel 11** (Strangler Fig).
- Reemplazar `ReportesController` (6900 líneas) por servicios por dominio + jobs en cola para reportes pesados (Laravel Queues / Horizon).
- Implementar permisos row-level (Spatie Laravel Permission + Policies).
- API v2 con OpenAPI + JWT + rate-limit Redis.
- 2FA obligatorio para roles admin.
- Auditoría externa de penetración anual.
- Plan de respuesta a incidentes y notificación a la SIC (Habeas Data, art. 17).

---

## 9. Checklist consolidado de seguridad

### Autenticación y sesión
- [ ] Eliminado backdoor `getPassAccess` + `getUserAccess` + `MysqlPDO::validateUser`.
- [ ] `password_hash` Argon2id, `password_verify` time-safe, re-hash perezoso.
- [ ] `session_regenerate_id(true)` en login.
- [ ] `session_destroy()` en logout + cookie borrada.
- [ ] Cookies `HttpOnly`, `Secure`, `SameSite`.
- [ ] Timeout 30 min de inactividad.
- [ ] MFA / SSO obligatorio para admins.
- [ ] Lockout y throttling en login.
- [ ] Mensaje genérico "Credenciales inválidas".

### OAuth Microsoft
- [ ] `state` y `nonce` aleatorios, validados.
- [ ] Validación de `id_token` con JWKS (firma, `iss`, `aud`, `tid`, `exp`, `nonce`).
- [ ] Single-tenant o whitelist de tenants.
- [ ] Librería auditada (`thenetworg/oauth2-azure`).
- [ ] Redirect URI desde `.env` por entorno.
- [ ] `client_secret` sólo en `.env`.

### Inyección
- [ ] Prepared statements (`SafePDO`) en TODAS las queries.
- [ ] Whitelist de columnas para `ORDER BY`/`GROUP BY`.
- [ ] Eliminar `queryAllSql($sql)` con interpolación.
- [ ] Eliminar `addslashes` como defensa.
- [ ] Migraciones DDL desde CLI con usuario separado.

### XSS
- [ ] Helper `e()` y reemplazo en vistas.
- [ ] CSP estricta con nonces.
- [ ] Eliminar `echo $exception`.
- [ ] No emitir `<script>alert(...)</script>` con datos de usuario.

### CSRF
- [ ] Token en sesión, `hash_equals`.
- [ ] Verificación en POST/PUT/DELETE/PATCH.
- [ ] Header `X-CSRF-Token` en AJAX.
- [ ] Cookies `SameSite`.

### Subida de archivos
- [ ] `files/` fuera del webroot.
- [ ] `finfo_file` + whitelist MIME real.
- [ ] Renombrado UUID + extensión controlada.
- [ ] Límite de tamaño.
- [ ] `.htaccess` bloqueando ejecución (defensa en profundidad).
- [ ] ClamAV en pipeline.
- [ ] Descargas vía controller con auth.

### APIs
- [ ] Auth obligatoria (`@` o JWT).
- [ ] Rate-limit por IP + token.
- [ ] Versionado (`/api/v1/...`).
- [ ] Documentación OpenAPI.
- [ ] Paginación obligatoria.
- [ ] CORS restrictivo.

### Secretos
- [ ] `.env` fuera del repo.
- [ ] Rotación inmediata de claves comprometidas.
- [ ] Pre-commit `gitleaks`.
- [ ] Historial git purgado.
- [ ] Hard cap de gasto en OpenAI.

### HTTP / Apache
- [ ] HTTPS forzado + HSTS.
- [ ] Headers CSP, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy.
- [ ] `expose_php = Off`, `display_errors = Off`, `log_errors = On`.
- [ ] WAF (Cloudflare o ModSecurity + OWASP CRS).
- [ ] Scripts de debug eliminados.
- [ ] Apache: `ServerTokens Prod`, `ServerSignature Off`.

### Aplicativo
- [ ] Row-level security (alcance por sede/rol/usuario) en todos los `getById`/`deleteById`.
- [ ] `classesGeneral` reducida; AJAX bajo permisos.
- [ ] Logs de auditoría firmados (HMAC).
- [ ] PII filtrada por rol en exports/reportes.

### Datos / Cumplimiento
- [ ] Cifrado de columnas PII en reposo.
- [ ] Backups diarios cifrados.
- [ ] Restauración probada mensualmente.
- [ ] Política de retención documentada.
- [ ] Aviso de privacidad y consentimientos (Habeas Data, Ley 1581).
- [ ] Procedimiento de notificación de incidentes a la SIC (≤ 15 días hábiles).

### Proceso / DevOps
- [ ] Pull Requests obligatorios + 1 reviewer.
- [ ] CI: PHPStan ≥ 6, Psalm, gitleaks, composer audit, PHPUnit.
- [ ] Deploy con Deployer (releases atomicos, rollback).
- [ ] Tags firmados en releases.
- [ ] Migraciones versionadas.
- [ ] Entornos separados (dev/staging/prod) con datos enmascarados en no-prod.

---

## 10. Plan de remediación sugerido

### Sprint 0 (esta semana — emergencia)
- Quick wins 1–17 (sección 7).
- Comunicar a equipo el estado de claves rotadas.

### Sprint 1–2
- `SafePDO` + migración de endpoints públicos/AJAX (C4, C8).
- CSRF global (C5).
- OAuth con librería + JWKS (C6).
- Helper de upload + mover `files/` (C7).

### Sprint 3–4
- Lockout + CAPTCHA + 2FA admin (C13).
- Helper `e()` + barrido XSS (C9).
- Row-level scoping en módulos críticos (C10).
- Cifrado de PII en reposo.
- WAF + CSP estricta.

### Trimestre 2
- Refactor `ReportesController` por dominios + jobs en cola.
- Migrar logging/auditoría firmada.
- CI/CD con Deployer y PRs obligatorios.

### Año 1
- Migración a Laravel 11 (Strangler Fig).
- Pentest externo + cierre de hallazgos.
- Certificación interna de cumplimiento Habeas Data.

---

## 11. Anexo — Snippets de mitigación listos para usar

### 11.1 `.env` mínimo
```
APP_ENV=production
APP_DEBUG=false
APP_URL=https://polimero.poligran.edu.co

DB_DRIVER=mysql
DB_HOST=127.0.0.1
DB_NAME=klee_planner
DB_USER=klee_app
DB_PASSWORD=__rotada__

MS_TENANT_ID=dd505be5-ec69-47f5-92df-caa55febf5fa
MS_CLIENT_ID=__nuevo__
MS_CLIENT_SECRET=__nuevo__
MS_REDIRECT_URI=https://polimero.poligran.edu.co/public/?c=login&a=validated

OPENAI_API_KEY=__nueva__

SMTP_HOST=smtp.office365.com
SMTP_PORT=587
SMTP_USER=__rotada__
SMTP_PASSWORD=__rotada__

APP_KEY=__base64_32_bytes__   # para sodium_crypto_secretbox
CSRF_SECRET=__base64_32_bytes__
```

### 11.2 `.gitignore`
```
.env
.env.*
!.env.example
files/**/*
!files/.gitkeep
*.log
vendor/
node_modules/
```

### 11.3 Rotación de password MD5 → Argon2id (one-shot)
```php
// scripts/migrate_passwords.php — ejecutar 1 sola vez
$users = UsuariosModel::queryAllSql("SELECT Id, Contrasena FROM usuarios WHERE LENGTH(Contrasena) = 32");
foreach ($users as $u) {
    // No conocemos la contraseña; marcamos para rehash al próximo login.
    // En el login: si MD5 coincide, se rehashea.
}
```

### 11.4 Apache `.htaccess` raíz endurecido
```apache
RewriteEngine On
# Forzar HTTPS
RewriteCond %{HTTPS} !=on
RewriteRule ^ https://%{HTTP_HOST}%{REQUEST_URI} [L,R=301]

# Headers de seguridad
Header always set Strict-Transport-Security "max-age=31536000; includeSubDomains"
Header always set X-Frame-Options "DENY"
Header always set X-Content-Type-Options "nosniff"
Header always set Referrer-Policy "strict-origin-when-cross-origin"
Header always set Permissions-Policy "geolocation=(), microphone=(), camera=()"
Header always set Content-Security-Policy "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self'; object-src 'none'; base-uri 'self'; frame-ancestors 'none'"
Header unset X-Powered-By
ServerSignature Off

# Bloquear acceso a archivos sensibles
<FilesMatch "(^\.env|composer\.(json|lock)|package(-lock)?\.json|\.git.*|\.htaccess|.*\.sql|.*\.md)$">
    Require all denied
</FilesMatch>
```

### 11.5 `/files/.htaccess`
```apache
<FilesMatch "\.(php|phtml|phar|phps|pl|py|cgi|sh|asp|aspx|htaccess|svg)$">
    Require all denied
</FilesMatch>
Options -ExecCGI -Indexes -FollowSymLinks
RemoveHandler .php .phtml .phar
AddType text/plain .php .phtml .phar
SetHandler None
```

---

## 12. Estimación de severidad agregada

| Severidad | Cantidad |
|---|---|
| 🔴 Crítica | 7 (C1, C2, C3, C4, C5, C6, C7) |
| 🟠 Alta | 8 (C8, C9, C10, C11, C12, M4, M6, M14, M15, M16, M17) |
| 🟡 Media | ~9 (C13, M1, M2, M5, M7, M10, M11, M12, M13) |
| 🔵 Baja | 3 (M3, M8, M9) |

> **Veredicto del auditor:** el sistema procesa información personal de talento humano y opera sin las salvaguardas mínimas exigibles (OWASP Top 10 2021 ASVS L1). Recomiendo **detener la incorporación de nuevos módulos** hasta cerrar Sprint 0–2 del plan de remediación. La organización debería evaluar reportar internamente esta auditoría a su responsable de tratamiento de datos personales y, ante cualquier incidente, considerar notificación a la SIC dentro de los plazos legales.

---

*Fin del informe — auditoría preparada por Copilot (revisión técnica white-box).*