diff --git a/docs/TRACKING.md b/docs/TRACKING.md index 0292639..937ed50 100644 --- a/docs/TRACKING.md +++ b/docs/TRACKING.md @@ -114,7 +114,7 @@ risk as core `mod_scorm` and any client-graded SCORM player; use a server-graded | 5 | Grade another user | Spoof a userid in the payload | The payload carries no userid; `ingest()` is always called with `$USER->id` (web `track.php:66`, WS `save_track.php:137`) | `track.php:66`; `classes/external/save_track.php:137` | | 6 | CSRF on the tracking endpoint | Cross-site POST to `track.php` | The session key is confirmed before any work. It is carried in the JSON body, not the query string, so access logs and proxies never record it (SEC-04) | `track.php:51`; `classes/local/tracking_endpoint.php` | | 7 | Unauthorised save | Unauthenticated / unprivileged POST | `require_login($course,…,$cm)` then `require_capability('mod/exelearning:savetrack')` (preview path needs `moodle/course:manageactivities`); WS adds `validate_context()` + same capability | `track.php:46,49-55`; `save_track.php:105-107` | -| 8 | Malicious package navigates parent / spams modals | iDevice JS tries `top.location` / `alert()` | **Not enforceable.** The `sandbox` omits `allow-top-navigation` and `allow-modals`, but it also grants `allow-same-origin` next to `allow-scripts`, so package JS can reach `parent`/`top` (same origin, unsandboxed realm) and call `parent.alert()`, set `parent.location`, or rewrite the Moodle page. The omissions only stop the naive in-frame calls. Accepted residual risk; the real fix is a separate origin (RIE-001, see below) | `view.php:404-413`; rationale `research/analisis/notas/AN-008-iframe-vs-scorm-player.md:116-153` | +| 8 | Malicious package navigates parent / spams modals | iDevice JS tries `top.location` / `alert()` | **Not enforceable.** The `sandbox` omits `allow-top-navigation` and `allow-modals`, but it also grants `allow-same-origin` next to `allow-scripts`, so package JS can reach `parent`/`top` (same origin, unsandboxed realm) and call `parent.alert()`, set `parent.location`, or rewrite the Moodle page. The omissions only stop the naive in-frame calls. Accepted residual risk; the real fix is a separate origin (RIE-001, see below) | `view.php:388-412`; rationale `research/analisis/notas/AN-008-iframe-vs-scorm-player.md:116-153` | | 9 | Status-only commit recorded as a real 0 | Mobile sends a status update with no score | `scoreraw` is nullable; omitting it skips `cmi.core.score.raw`, so `ingest()` no-ops instead of persisting a 0-score attempt (DEC-34-01 / B6) | `save_track.php:60-66,121-129`; no-op guard `classes/local/track.php:79-82` | | 10 | Package HTML opened top-level, outside the iframe | Learner (or a link inside the package) opens a `content/` file URL directly | **None at serve time.** `exelearning_pluginfile()` serves the `content` area to anyone with `mod/exelearning:view` as a same-origin document with no sandbox and no CSP (SVG inline too), so the iframe sandbox does not apply. Package JS then runs with the viewer's full Moodle session. Today's only control is **who can upload**: `mod/exelearning:addinstance` and `moodle/course:manageactivities` carry `RISK_XSS`, the same trust model as `mod_scorm`/`mod_resource`. The real fix is serving `content/` from a separate origin (RIE-001 / DEC-0-16) | `lib.php:522-580`; `db/access.php:38-46` | @@ -131,6 +131,15 @@ is the boundary. Cross-component XSS hardening `allow-popups-to-escape-sandbox`) is roadmapped as **RIE-001** / **DEC-0-16** — see `research/analisis/notas/AN-008-iframe-vs-scorm-player.md:124-153`. +The sandbox also grants `allow-downloads`. Without it the browser silently drops +every download the frame starts: `` links and the download-source-file +iDevice's "Download .elpx" button, which rebuilds the package in the browser +(exelearning/exelearning#2488). It adds nothing to threat 8, because same-origin package +script can already start a download through the parent. Any future CSP for the +`content` area (DEC-0-16 M3) must also allow `worker-src 'self' blob:`. Otherwise that +button's fflate compression cannot start its workers, and in packages exported before +exelearning/exelearning#2489 it hangs at "Processing... 100%". + ## What is, and is not, tech debt The SCORM 1.2 `window.API` shim in `view.php` is **not** considered tech debt: it is diff --git a/tests/behat/mod_exelearning.feature b/tests/behat/mod_exelearning.feature index 6d892a4..36fe4cb 100644 --- a/tests/behat/mod_exelearning.feature +++ b/tests/behat/mod_exelearning.feature @@ -182,6 +182,19 @@ Feature: View a mod_exelearning activity and its attempts report And I am on the "Teacher noreveal" "exelearning activity" page logged in as teacher1 Then the "src" attribute of "iframe#exelearningobject" "css_element" should not contain "exe-teacher" + # The package's own download links, including the download-source-file iDevice's + # "Download .elpx" button, need allow-downloads or the browser drops the file + # (exelearning/exelearning#2488). The attribute is server-rendered, so the non-JS + # driver asserts on it directly. + Scenario: The package iframe lets the package download files + Given the following "activities" exist: + | activity | name | course | idnumber | + | exelearning | Download frame | C1 | exedl | + And I am on the "Download frame" "exelearning activity" page logged in as student1 + Then the "sandbox" attribute of "iframe#exelearningobject" "css_element" should contain "allow-downloads" + And the "sandbox" attribute of "iframe#exelearningobject" "css_element" should not contain "allow-top-navigation" + And the "sandbox" attribute of "iframe#exelearningobject" "css_element" should not contain "allow-modals" + Scenario: A student also sees the exe-teacher parameter when the setting is on Given the following "activities" exist: | activity | name | course | idnumber | teachermodevisible | diff --git a/version.php b/version.php index a90fa12..3657678 100644 --- a/version.php +++ b/version.php @@ -34,7 +34,7 @@ // in $plugin->release ('dev'); a release-preparation PR commits the final // version + semver release BEFORE the tag is created (see DEVELOPMENT.md, // "Versioning and releases"). -$plugin->version = 2026092610; +$plugin->version = 2026092900; $plugin->release = 'dev'; $plugin->requires = 2024100700; // Moodle 4.5 LTS+. $plugin->supported = [405, 502]; // Moodle 4.5 LTS through Moodle 5.2. diff --git a/view.php b/view.php index 89d0550..2a247b5 100644 --- a/view.php +++ b/view.php @@ -391,6 +391,11 @@ // allow-popups: interactive-video, hidden-image, etc. // allow-forms: quick-questions, form, scrambled-list, etc. // allow-popups-to-escape-sandbox: popups load without restrictions. + // allow-downloads: links and the download-source-file iDevice's + // "Download .elpx" button, which rebuilds the package in the browser and saves it. + // Without it Chrome/Firefox silently drop every download the frame starts + // (exelearning/exelearning#2488). Same-origin script can already trigger + // downloads through the parent, so this grants no new capability. // Explicitly BLOCKED (not included): // allow-top-navigation: a malicious package must not change the parent URL. // allow-modals: no alert/confirm/prompt, they are UX interruptions. @@ -402,7 +407,7 @@ 'width' => '100%', 'height' => '650', 'allow' => 'fullscreen', - 'sandbox' => 'allow-scripts allow-same-origin allow-popups allow-forms allow-popups-to-escape-sandbox', + 'sandbox' => 'allow-scripts allow-same-origin allow-popups allow-forms allow-popups-to-escape-sandbox allow-downloads', 'style' => 'border: 1px solid var(--bs-border-color, #dee2e6); border-radius: .5rem;', ]);