Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion docs/TRACKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` |

Expand All @@ -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: `<a download>` 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
Expand Down
13 changes: 13 additions & 0 deletions tests/behat/mod_exelearning.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
2 changes: 1 addition & 1 deletion version.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion view.php
Original file line number Diff line number Diff line change
Expand Up @@ -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: <a download> 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.
Expand All @@ -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;',
]);

Expand Down
Loading