chore: cherry-pick security fixes - #855
Open
kaustavb12 wants to merge 10 commits into
Open
Conversation
cherry-pick from upstream: openedx#38241 The activation_key field was exposed in /api/user/v1/accounts/{username}, allowing an attacker to bypass email verification by combining two behaviors: 1. OAuth2 password grant issues tokens to inactive users (intentional) 2. activation_key returned in API response (the vulnerability) An attacker could register, get an OAuth2 token, read the activation_key from the API, then GET /activate/{key} to activate without email access. Fix: remove activation_key from UserReadOnlySerializer.to_representation() and from ACCOUNT_VISIBILITY_CONFIGURATION["admin_fields"] (which controls the field whitelist in _filter_fields — listed fields default to None even if absent from the serializer data dict). Reported by Daniel Baillo via the Open edX security working group. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> (cherry picked from commit bb84543)
cherry-pick from openedx#38274 The view_survey endpoint accepted a redirect_url GET parameter and passed it directly to HttpResponseRedirect() with no validation. If a non-existent survey name was requested, this produced an immediate 302 to an attacker-controlled URL. If a valid survey was requested, the same URL was embedded in a hidden _redirect_url form field; after submission, submit_answers echoed it back in JSON and client-side JS used it as location.href — a second unvalidated redirect path. Fix both by ignoring user-supplied redirect URLs entirely: - view_survey no longer reads redirect_url from GET params - submit_answers always redirects to reverse('dashboard') rather than reading _redirect_url from the POST body Note: view_student_survey retains its redirect_url parameter because it is also called from the courseware view (courseware/views/views.py), which passes a server-controlled course_home_url. That call path is unaffected. Fixes: GHSA-2843-x998-f8r2 BREAKING CHANGE: The redirect_url GET parameter on /survey/<name>/ is no longer honored. Requests that previously redirected to a caller-specified URL after survey completion will now always redirect to the dashboard. (cherry picked from commit 5d94b66)
The set_course_mode_price view had no authorization check beyond @login_required, meaning any authenticated user could POST to it and rewrite the honor-mode price for any course — a privilege escalation vulnerability. Ideally this endpoint would be removed: it has no known callers in the UI (no templates or JS reference it), no tests, and targets the legacy 'honor' mode. However, it is publicly routed and external consumers may depend on it, so removal requires going through the DEPR process before we can act. In the meantime, this commit closes the security hole regardless of how active the endpoint is. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> (cherry picked from commit fd93ef5) (cherry picked from commit 1f1346f)
validate_timestamp_and_nonce previously returned True unconditionally, allowing any captured LTI launch request to be replayed indefinitely. Now rejects requests whose oauth_timestamp falls outside a ±5-minute window, then atomically records the nonce in the Django cache via cache.add() (OEP-0022 key generation via get_cache_key). A replay returns False immediately because cache.add() only writes when the key is absent. TieredCache is intentionally not used here: it has no atomic add primitive, so a separate get-then-set would leave a race window that defeats the protection. See the updated docstring for details. Documents the requirement for a shared cache backend (Redis or Memcached) in multi-node deployments in both the app and repo READMEs. Fixes GHSA-6gm5-c49g-p3h9 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> (cherry picked from commit 0a92cf2) (cherry picked from commit 69336c4)
The PUT /api/contentstore/v1/videos/{course_id}/download endpoint fetched
every client-supplied files[].url server-side with
requests.get(url, allow_redirects=True) and returned the bytes inside the
ZIP response. Because the URLs were never validated, an authenticated user
with studio read access could point them at internal services or cloud
metadata endpoints and exfiltrate the responses (GHSA-fpf9-9rpr-jvrx).
By design these URLs are always a subset of the course's own VAL
encoded_videos[].url values (the same data the video listing hands the
frontend). Restrict fetches to that allowlist: build the set of legitimate
URLs for the course and reject any request containing a URL outside it
before any HTTP request is made. This eliminates the SSRF rather than
merely narrowing it.
Adds VideoDownloadViewTest (the endpoint previously had no test coverage)
covering the allowed-URL success path, rejection of disallowed URLs without
any outbound request, mixed allowed/disallowed batches, and the non-staff
permission gate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 241b914)
(cherry picked from commit 30f277f)
(cherry picked from commit f5bc134)
The previous vendored pdf.js was 1.0.907 (May 2013), four major versions behind upstream and within the range covered by Mozilla's GHSA-wgrm-67xf-hhpq (arbitrary JavaScript execution upon opening a malicious PDF). 5.7.284 is well past the >= 4.2.67 fix line. The replacement comes from Mozilla's prebuilt `pdfjs-5.7.284-legacy-dist.zip` GitHub Release artifact rather than the `pdfjs-dist` npm package because the npm package is library-only -- it ships `pdf.mjs` plus a bare `PDFViewer` component class, but no `viewer.html` / `viewer.mjs` / `viewer.css` / locale files. A full npm integration would mean rewriting the viewer page against the bare component, which is appropriate as a non-security follow-up but not as the fix here. The viewer page (`lms/templates/pdf_viewer.html`) is rewritten as a Mako adaptation of upstream `web/viewer.html`. A `<base href>` makes the viewer's relative asset URLs resolve against the vendored copy. The analytics shim (`lms/static/js/pdf-analytics.js`) is rewritten in vanilla JS against `PDFViewerApplication.eventBus`. Four analytics events (`textbook.pdf.thumbnails.toggled`, `textbook.pdf.thumbnail.navigated`, `textbook.pdf.outline.toggled`, `textbook.pdf.page.scrolled`) no longer fire because the corresponding UI elements were refactored away in pdf.js 4.x's Views Manager redesign. A new `scripts/refresh-pdfjs-vendor.sh` is the tool for future bumps: update PDFJS_VERSION + PDFJS_LEGACY_ZIP_SHA256, re-run, commit. Closes GHSA-mj74-gfq3-2v9f. (cherry picked from commit 4d2e220) Co-authored-by: Feanil Patel <feanil@axim.org> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit cd63453)
Discussion thread titles (and other user-controlled context fields — replier_name, author_name, username) were interpolated raw into notification.content via `str.format(**context)`. That output is rendered with Django's `|safe` filter in digest_content.html, which is included by both the email_digest and batched_email body templates, so a `<style>` block in a thread title survived into recipient inboxes as executable CSS on email open — enabling open-tracking, content spoofing, and phishing. Escape at the source: in `get_notification_content`, wrap every context value with `django.utils.html.escape` before `template.format(**context)`, exempting the two structural keys (`p`, `strong`) that content_templates use as HTML tag names. This defends every renderer of `notification.content` in one place. This is the incomplete-patch companion of GHSA-4xv3-5j4x-q8g4 (CVE-2026-42857), which sanitized the post body via `clean_thread_html_body()` but did not cover the title path. Fixes GHSA-rv5w-f4r5-h77g. (cherry picked from commit 08b719ce41bb369fa0cabbe8d0547124e64c8566) (cherry picked from commit 52f2b3b) (cherry picked from commit 526dc36)
`startswith` is the wrong primitive for "is target inside directory base": once a trailing separator drops anywhere along the way, sibling directories whose names extend base match. We could spot-fix by re-appending the separator before the check, but `commonpath` makes the directory-boundary intent explicit and removes the failure mode entirely. Fixes GHSA-6cmm-8875-5pcw. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit 97de058) (cherry picked from commit c92a839)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR cherry picks security fixes from the following PRs:
Other information
BB-10730