# Variant RCA Report — GHSA-h9qx-v5xp-ph8p (EGroupware isParticipant() Authz Bypass)

## Summary

The official fix for GHSA-h9qx-v5xp-ph8p (EGroupware tags `26.2.20260224` /
`23.1.20260224`) hardened **only** the `SmallPartMediaRecorder::ajax_upload()`
caller by casting `course_id` to `(int)` and by adding `basename()` guards in
`Bo::videoPath()`. It did **NOT** fix the root cause: `Bo::isParticipant()`
still trusts a request-supplied `participants` array to cache the caller's ACL
role, allowing any authenticated user to assert an arbitrary role (teacher,
admin) for any course. This script confirms that the same authz-bypass
primitive still works on the **fixed** image (`26.2.20260224`) via a different
entry point — `Overlay::ajax_write()` → `Overlay::aclCheck()` →
`Bo::isTeacher()` / `Bo::isParticipant()`. On the fixed image, a non-teacher
user sending `course_id` as a PHP array with `participants[].participant_role=3`
bypasses the `aclCheck` (the response changes from "Permission denied!" to
"Invalid argument", the latter thrown by `Overlay::write()`'s pre-existing
type guard — proving the authz check was passed). The `isParticipant()`
function is unchanged from `26.2.20260216` through the latest tag
`26.7.20260702`, so the bypass persists across all released versions.

## Fix Coverage / Assumptions

### What the fix relies on
The fix assumes that the **only** path reaching `isTeacher()` with
attacker-controlled data is `SmallPartMediaRecorder::ajax_upload()`, and that
hardening that single caller (via `(int)` cast) is sufficient.

### What the fix explicitly covers
1. **`SmallPartMediaRecorder::ajax_upload()`** (`src/Widgets/SmallPartMediaRecorder.php`):
   - `$bo->isTeacher((int)$data['video']['course_id'])` — casts `course_id` to
     int, defeating the `participants`-array bypass (a non-empty array casts to
     `1`, and `isTeacher(1)` reads real ACL from the DB).
   - `$video = $bo->readVideo((int)$data['video']['video_id'])` — requires a
     real DB video row; the path is then derived from the DB record, not the
     request.
   - `$filePath = $bo->videoPath($video, true)` — uses DB-sourced `$video`.

2. **`Bo::videoPath()`** (`src/Bo.php`):
   - `basename($video['video_hash']) !== $video['video_hash']` — rejects path
     traversal in `video_hash`.
   - `basename($video['video_type']) !== $video['video_type']` — rejects path
     traversal in `video_type`.

3. **`importexport_export_ui::download()`** (egroupware core,
   `importexport/inc/class.importexport_export_ui.inc.php`):
   - `basename($tmpfname) !== $tmpfname` — rejects path traversal in the
     `_filename` parameter (the arbitrary-file-read primitive).

### What the fix does NOT cover
- **`Bo::isParticipant()` itself** — the function still honors a
  request-supplied `participants` array and caches the attacker-supplied role
  in `$this->course_acl[$course['course_id']]`. The fix did not change this
  function at all.
- **All other callers of `isTeacher()` / `isParticipant()` / `isAdmin()` /
  `isTutor()` / `isStaff()`** that pass request-controlled data without casting
  to `(int)`. These include:
  - `Overlay::aclCheck($course_id, true)` — called from `Overlay::ajax_write()`
    and `Overlay::ajax_delete()` with raw request `$data['course_id']`.
  - `Student\Ui::ajax_listComments()` — `isTeacher($where)` with raw request
    `$where`.
  - `Student\Ui::showCommentButton()` / `showNoteButton()` — `isTeacher($content)`
    with etemplate form data.
  - `Bo::save()` — `isTeacher($keys['course_id'])` with etemplate form data
    (course modification path).
  - `Bo::close()` — `isAdmin($id)` with request data (via `Courses::ajax_action`).
- The latest tag `26.7.20260702` also does **not** fix `isParticipant()`.

## Variant / Alternate Trigger

### Primary variant: `Overlay::ajax_write()` authz bypass

**Entry point:** `POST /egroupware/json.php?menuaction=EGroupware\SmallParT\Overlay::ajax_write`
with a JSON body:
```json
{"request":{"parameters":[
  {"course_id":{"participants":[{"account_id":7,"participant_role":3}],
    "account_id":"7","course_id":"1"},
   "video_id":1,"overlay_type":"smallpart-text","overlay_start":0,"overlay_data":"{}"}
]}}
```

**Code path:**
1. `Overlay::ajax_write(array $data)` → `self::aclCheck($data['course_id'], true)`
2. `Overlay::aclCheck($course_id, $update=true)`:
   - `Bo::getInstance()->isParticipant($course_id)` — `$course_id` is the array.
     `isParticipant()` sees `is_array($course) && isset($course['participants'])`,
     filters for the attacker's `account_id`, caches
     `$this->course_acl["1"] = 3` (ROLE_TEACHER), and returns `true`.
   - `$update && !Bo::getInstance()->isTeacher($course_id)` — `isTeacher()`
     calls `isParticipant($course_id, ROLE_TEACHER)`, finds the just-cached
     value `3 & 3 === 3`, returns `true`. The `aclCheck` **passes**.
3. `Overlay::write($data)` — has a **pre-existing** type guard
   `is_int($data['course_id']) || is_numeric($data['course_id'])` that rejects
   the array with `InvalidArgumentException("Invalid argument...")`.

**Observable proof:**
- **Control** (plain int `course_id=1`, non-teacher user): response =
  `{"message":"Permission denied!","type":"error"}` — `aclCheck` correctly
  denies.
- **Bypass** (array `course_id` with `participants`): response =
  `{"message":"Invalid argument EGroupware\\SmallParT\\Overlay::write({...})",
  "type":"error"}` — `aclCheck` **passed** (the request reached `write()`'s
  type guard). The different error proves the authz bypass.

### Secondary variant candidates (analyzed, not all runtime-tested)

| # | Entry point | Caller | `isTeacher`/`isParticipant` arg | Downstream guard | Exploitable? |
|---|------------|--------|------|------|-------------|
| 1 | `Overlay::ajax_write` | `aclCheck($data['course_id'], true)` | raw request array | `write()`: `is_int\|\|is_numeric` | Authz bypass **confirmed** on fixed; write blocked by pre-existing type guard |
| 2 | `Overlay::ajax_delete` | `aclCheck($what['course_id'], true)` | raw request array | `delete()`: `is_int` | Same as #1 — authz bypass works, delete blocked by type guard |
| 3 | `Student\Ui::ajax_listComments` | `isTeacher($where)` | raw request array | `listComments()` has own ACL via `read()` (DB-sourced) | Authz bypass of `isTeacher` works, but `listComments` blocks via DB-sourced `read()` |
| 4 | `Bo::save()` via `Courses::edit()` | `isTeacher($keys['course_id'])` | etemplate form data (array) | `data2db()` / `so->save()` | Authz bypass works; course modification possible but requires etemplate `exec_id` |
| 5 | `Bo::close()` via `Courses::ajax_action('close')` | `isAdmin($id)` | raw request array | `so->close()` with array WHERE | Authz bypass works; `so->close()` may fail with array input |

## Impact

- **Package / component affected:** `egroupware/egroupware` (bundled `smallpart`
  app, `src/Bo.php` `isParticipant()` function). The root cause is in the
  shared ACL function, affecting all smallpart AJAX endpoints that call
  `isTeacher()` / `isParticipant()` / `isAdmin()` with request-controlled data.
- **Affected versions:** `26.2.20260216` through **latest** `26.7.20260702`
  (and `23.1` branch). The `isParticipant()` function was never changed.
- **Risk level:** The authz bypass itself is confirmed on the fixed version.
  The security impact varies by caller:
  - `Overlay::ajax_write/delete`: authz bypass confirmed, but the downstream
    `write()`/`delete()` pre-existing type guards (`is_int`/`is_numeric`)
    block the array, preventing the actual write/delete. The authz check is
    bypassed but the operation doesn't complete.
  - `Bo::save()` via `Courses::edit()`: would allow a student to modify a
    course (change password, add themselves as teacher/admin) — a privilege
    escalation. Requires etemplate `exec_id` (attainable if the student is a
    participant of the course).
  - The **original RCE** (file write to `header.inc.php`) is **NOT** achievable
    through these variant paths because `Bo::videoPath()` now has `basename()`
    guards and the `ajax_upload` caller uses DB-sourced data.

## Impact Parity

- **Disclosed/claimed maximum impact (parent):** Remote Code Execution via
  authorization bypass + arbitrary file write + arbitrary file read → full
  system compromise.
- **Reproduced impact (this variant):** Authorization bypass confirmed on the
  fixed version — `isParticipant()` / `isTeacher()` can be bypassed via
  request-supplied `participants` arrays from a different entry point
  (`Overlay::ajax_write`). The bypass reaches past the ACL check but is blocked
  by a pre-existing type guard in the downstream `write()` method.
- **Parity:** `partial` — the authz-bypass primitive (the root cause) is
  reproduced on the fixed version, but the full RCE chain is NOT achieved
  through this variant because `videoPath()` now has `basename()` guards.
- **Not demonstrated:** RCE or arbitrary file write via the variant paths.
  The variant demonstrates that the **root cause** (isParticipant trusting
  request data) is unfixed, but the **specific RCE sink** (file write via
  `videoPath()` traversal) is properly closed by the `basename()` guards.

## Root Cause

`Bo::isParticipant($course, int $required_acl=0, bool $check_agreed=false)` in
`smallpart/src/Bo.php` (lines ~2339-2380) contains:

```php
if (is_array($course) && isset($course['participants']))
{
    $user = $this->user;
    $participants = array_filter($course['participants'], static function($participant) use ($user)
    {
        return is_array($participant) && $participant['account_id'] == $user && !isset($participant['participant_unsubscribed']);
    });
    $this->course_acl[$course['course_id']] = $participants ? current($participants)['participant_role'] : null;
}
```

When `$course` is a PHP array with a `participants` key, the function caches
the attacker-supplied `participant_role` in `$this->course_acl` and then uses
that cached value instead of reading from the database. Any authenticated user
can assert any role (student, tutor, teacher, admin) for any course by
supplying a crafted `participants` array.

The fix commit (tag `26.2.20260224`, smallpart commit
`b8754b6b8ec41d94807e2eecbeded3dc8b7f834a`) changed only:
- `src/Widgets/SmallPartMediaRecorder.php` — `(int)` cast on `course_id` +
  `readVideo()` for DB-sourced path
- `src/Bo.php` — `basename()` guards in `videoPath()`

It did **not** modify `isParticipant()`, `isTeacher()`, `isAdmin()`, or any
other caller. The `isParticipant()` function is identical across `26.2.20260216`
→ `26.2.20260224` → `26.7.20260702` (verified via `git diff`).

## Reproduction Steps

1. **Reference script:** `bundle/vuln_variant/reproduction_steps.sh`
2. **What the script does:**
   - Pulls `egroupware/egroupware:26.2.20260216` (vulnerable) and
     `:26.2.20260224` (fixed), `mariadb:11.8`, `nginx:stable-alpine`.
   - For each version, deploys a 3-container stack (mariadb + egroupware
     php-fpm + nginx) on an isolated Docker network with `EGW_DB_GRANT_HOST=%`.
   - Creates a low-privileged, non-teacher, non-admin user `attacker` directly
     in `egw_accounts` (member of the `Default` group only, with `smallpart`
     app `run` ACL).
   - Logs in as `attacker` via `POST /egroupware/login.php`.
   - **Control:** sends `Overlay::ajax_write` with a plain integer `course_id`
     → expects "Permission denied!" (aclCheck denies non-teacher).
   - **Bypass:** sends `Overlay::ajax_write` with `course_id` as a PHP array
     containing `participants[].participant_role=3` → expects NOT "Permission
     denied!" (proves the authz check was bypassed; the response is "Invalid
     argument" from `write()`'s type guard).
   - Compares results across both versions.
3. **Expected evidence:**
   - `bundle/logs/fixed_overlay_write_control.json` — "Permission denied!"
   - `bundle/logs/fixed_overlay_write_bypass.json` — "Invalid argument" (NOT
     "Permission denied!" → authz bypass confirmed on fixed version)
   - `bundle/logs/vuln_overlay_write_control.json` — "Permission denied!"
   - `bundle/logs/vuln_overlay_write_bypass.json` — "Invalid argument" (same
     behavior on vulnerable version, since `isParticipant()` was never changed)

## Evidence

- `bundle/logs/fixed_overlay_write_control.json`:
  ```json
  {"response":[{"type":"message","data":{"message":"Permission denied!","type":"error"}}],"page_generation_time":"0.01"}
  ```
- `bundle/logs/fixed_overlay_write_bypass.json`:
  ```json
  {"response":[{"type":"message","data":{"message":"Invalid argument EGroupware\\SmallParT\\Overlay::write({\"course_id\":{\"participants\":[{\"account_id\":7,...\"participant_role\":3}],...})","type":"error"}}],...}
  ```
  The "Invalid argument" error (from `Overlay::write()` line 361) proves the
  request passed `aclCheck()` and reached `write()`. The `aclCheck` calls
  `isParticipant($course_id)` and `isTeacher($course_id)` with the raw request
  array — both returned `true` via the `participants`-array bypass.
- `bundle/logs/vuln_overlay_write_bypass.json` — identical behavior on the
  vulnerable version, confirming `isParticipant()` was never changed.
- `bundle/logs/vuln_variant_summary.txt` / `fixed_variant_summary.txt` —
  structured summaries.
- **Environment:** Docker; `egroupware/egroupware:26.2.20260216` and
  `:26.2.20260224` (PHP 8.5 fpm), `mariadb:11.8`, `nginx:stable-alpine`.
  All HTTP exercised through the real `nginx → php-fpm` boundary.
- **Source verification:** `isParticipant()` is byte-identical across
  `26.2.20260216` → `26.2.20260224` → `26.7.20260702` (verified via
  `git diff` and `git show` on the smallpart repo).

## Recommendations / Next Steps

1. **Fix the root cause in `Bo::isParticipant()`:** Remove the code path that
   honors a request-supplied `participants` array for authorization decisions.
   The `participants` array in the `$course` parameter is intended for
   performance optimization (avoiding a DB read when participant data is
   already available from a prior `read()` call), but it should NEVER be used
   when the data originates from untrusted request input. The fix should
   either:
   - Remove the `is_array($course) && isset($course['participants'])` cache
     population entirely and always read ACL from the DB, OR
   - Add a flag parameter to distinguish "internal call with DB-sourced data"
     from "external call with potentially request-sourced data", and only
     honor the `participants` array for internal calls.

2. **Harden all callers:** Cast `course_id` to `(int)` before passing to
   `isTeacher()` / `isParticipant()` / `isAdmin()` / `isTutor()` / `isStaff()`
   in ALL AJAX handlers and etemplate callbacks, not just `ajax_upload()`.
   Specifically:
   - `Overlay::aclCheck()` — cast `$course_id` to `(int)`.
   - `Student\Ui::ajax_listComments()` — cast `$where` components to `(int)`.
   - `Bo::save()` — cast `$keys['course_id']` to `(int)` before `isTeacher()`.
   - `Bo::close()` — cast `$id` to `(int)` before `isAdmin()`.

3. **Defense in depth:** Add type checks (`is_int` or `is_numeric`) at all
   AJAX entry points that receive `course_id` parameters, rejecting non-numeric
   values early.

## Additional Notes

- **Idempotency:** The script removes per-stack containers and named volumes
  (`docker rm -f` + `docker volume rm -f`) at the start of each deploy, so
  consecutive runs start from a clean install.
- **Why the bypass is confirmed but RCE is not:** The authz bypass
  (`isParticipant()` trusting request data) is the **root cause** shared
  between the original RCE chain and this variant. The fix addressed the
  **specific RCE sink** (file write via `videoPath()` traversal) with
  `basename()` guards, which properly prevents RCE even if the authz bypass
  works. However, the authz bypass itself enables other impacts (privilege
  escalation via `Bo::save()`, information disclosure via comment visibility,
  course state modification via `Bo::close()`) that the fix does not address.
- **Pre-existing type guards:** `Overlay::write()` and `Overlay::delete()`
  have `is_int`/`is_numeric` checks that predate the fix (present in
  `26.2.20260216`). These guards block the array `course_id` at the write/delete
  level, which is why the `Overlay::ajax_write` variant demonstrates the authz
  bypass but does not complete the write operation. These are NOT part of the
  security fix — they are incidental type guards that happen to block this
  specific variant.
- **Latest version check:** `isParticipant()` is unchanged in the latest tag
  `26.7.20260702` (verified via `git diff 26.2.20260224 26.7.20260702 --
  src/Bo.php`), so the bypass persists across all released versions.