From efa65b17fdbe60d518fbb03e623741afbe99c970 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 6 Aug 2026 09:44:40 -0500 Subject: [PATCH] Add csrf token protection to API calls. Anytime a page is rendered a csrf token is added to the `webworkConfig` variable. This is then retrieved in JavaScript anytime an API call is going to be performed and is attached to the request. The API will now refuse any request that does not have this token or if the token does not match the token in the session. This utilizes the Mojolicious `csrf_token` helper method. The point of this is that a user that has properly logged into webwork in the browser may be tricked into doing something in another tab or window by an attacker that would cause a request to be sent to the API. That request would automatically have the valid WeBWorK session cookie attached to it by the browser, and so the request would succeed and could do whatever the user has permission to do via the API. Adding the csrf token prevents that because the other tab or window would not have access to the `csrf_token` in the `webworkConfig` JavaScript variable. This was one of the vulnerabilities that @Alex-Jordan found with Claude before. This does mean that using the API via a script will not work as easily. You would have to extract the `csrf_token` either from the cookies in the response or by parsing the HTML to find the `csrf_token` in the `webworkConfig` variable. But it can still be done. If we really want a proper API for usage in scripts we could make it possible for users to generate an API token in the UI (those with sufficient permissions at least), and the API token added as an `Authorization: Bearer` header could be used as an alternative to the `csrf_token`. At this point I don't think this is needed though. I doubt anyone really uses the API in this way. Note that this does not apply to the `render_rpc` endpoint. It probably should, but that would cause issues for PreTeXt (although this could be worked around in much the same way that cookie disabling is done if the `allow_unsecured_rpc` option is set). At this point I think this is also not necessary as this endpoint generally can't do anything destructive. It just renders problems. --- htdocs/js/GatewayQuiz/gateway.js | 6 +++++- htdocs/js/PGProblemEditor/pgproblemeditor.js | 20 +++++++++++++++---- .../js/ProblemGrader/singleproblemgrader.js | 6 ++++-- htdocs/js/SetMaker/setmaker.js | 1 + htdocs/js/TagWidget/tagwidget.js | 1 + lib/WeBWorK/ContentGenerator.pm | 1 + lib/WeBWorK/ContentGenerator/API.pm | 3 +++ 7 files changed, 31 insertions(+), 7 deletions(-) diff --git a/htdocs/js/GatewayQuiz/gateway.js b/htdocs/js/GatewayQuiz/gateway.js index 2e7dbfe5a9..8e5ced80c6 100644 --- a/htdocs/js/GatewayQuiz/gateway.js +++ b/htdocs/js/GatewayQuiz/gateway.js @@ -167,7 +167,11 @@ const response = await fetch(`${webworkConfig?.webwork_url ?? '/webwork2'}/api/getCurrentServerTime`, { method: 'post', mode: 'same-origin', - body: new URLSearchParams({ ...authenParams, courseID: timerDiv.dataset.courseId }), + body: new URLSearchParams({ + ...authenParams, + courseID: timerDiv.dataset.courseId, + csrf_token: webworkConfig.csrf_token + }), signal: controller.signal }).catch(() => { /* Errors are ignored */ diff --git a/htdocs/js/PGProblemEditor/pgproblemeditor.js b/htdocs/js/PGProblemEditor/pgproblemeditor.js index 6c96fa53a2..129c419a16 100644 --- a/htdocs/js/PGProblemEditor/pgproblemeditor.js +++ b/htdocs/js/PGProblemEditor/pgproblemeditor.js @@ -129,7 +129,10 @@ // Send a request to the server to save the temporary file for the currently edited file. // This temporary file could be used for recovery, and is displayed if the page is reloaded. const saveTempFile = () => { - const request_object = { courseID: document.getElementsByName('courseID')[0]?.value }; + const request_object = { + courseID: document.getElementsByName('courseID')[0]?.value, + csrf_token: webworkConfig.csrf_token + }; const user = document.getElementsByName('user')[0]; if (user) request_object.user = user.value; @@ -212,7 +215,10 @@ // Send a request to the server to perltidy the current PG code in the CodeMirror editor. const tidyPGCode = () => { - const request_object = { courseID: document.getElementsByName('courseID')[0]?.value }; + const request_object = { + courseID: document.getElementsByName('courseID')[0]?.value, + csrf_token: webworkConfig.csrf_token + }; const user = document.getElementsByName('user')[0]; if (user) request_object.user = user.value; @@ -260,7 +266,10 @@ // Send a request to the server to convert the current PG code in the CodeMirror editor. const convertCodeToPGML = () => { - const request_object = { courseID: document.getElementsByName('courseID')[0]?.value }; + const request_object = { + courseID: document.getElementsByName('courseID')[0]?.value, + csrf_token: webworkConfig.csrf_token + }; const user = document.getElementsByName('user')[0]; if (user) request_object.user = user.value; @@ -302,7 +311,10 @@ // Send a request to the server to run the PG critic in the CodeMirror editor. const runPGCritic = () => { - const request_object = { courseID: document.getElementsByName('courseID')[0]?.value }; + const request_object = { + courseID: document.getElementsByName('courseID')[0]?.value, + csrf_token: webworkConfig.csrf_token + }; const user = document.getElementsByName('user')[0]; if (user) request_object.user = user.value; diff --git a/htdocs/js/ProblemGrader/singleproblemgrader.js b/htdocs/js/ProblemGrader/singleproblemgrader.js index 76355ad5bb..561eedbae3 100644 --- a/htdocs/js/ProblemGrader/singleproblemgrader.js +++ b/htdocs/js/ProblemGrader/singleproblemgrader.js @@ -127,7 +127,8 @@ problem_id: saveData.problemId, status: parseInt(scoreInput.value) / 100, ...(saveData.saveSubStatus === '1' ? { sub_status: parseInt(scoreInput.value) / 100 } : {}), - mark_graded: true + mark_graded: true, + csrf_token: webworkConfig.csrf_token }), signal: controller.signal } @@ -178,7 +179,8 @@ ...authenParams, courseID: saveData.courseId, answer_id: saveData.pastAnswerId, - comment_string: comment + comment_string: comment, + csrf_token: webworkConfig.csrf_token }), signal: controller.signal }); diff --git a/htdocs/js/SetMaker/setmaker.js b/htdocs/js/SetMaker/setmaker.js index 93b41df685..9e40b4248f 100644 --- a/htdocs/js/SetMaker/setmaker.js +++ b/htdocs/js/SetMaker/setmaker.js @@ -73,6 +73,7 @@ if (user) authenParams.user = user.value; const sessionKey = document.getElementsByName('key')[0]; if (sessionKey) authenParams.key = sessionKey.value; + authenParams.csrf_token = webworkConfig.csrf_token; return { library_name: 'Library', diff --git a/htdocs/js/TagWidget/tagwidget.js b/htdocs/js/TagWidget/tagwidget.js index 37fc6d8bc4..0c54299b9e 100644 --- a/htdocs/js/TagWidget/tagwidget.js +++ b/htdocs/js/TagWidget/tagwidget.js @@ -78,6 +78,7 @@ if (user) authenParams.user = user.value; const sessionKey = document.getElementsByName('key')[0]?.value; if (sessionKey) authenParams.key = sessionKey.value; + authenParams.csrf_token = webworkConfig.csrf_token; return { library_name: 'Library', diff --git a/lib/WeBWorK/ContentGenerator.pm b/lib/WeBWorK/ContentGenerator.pm index 8cb5f426d0..65f0d998fe 100644 --- a/lib/WeBWorK/ContentGenerator.pm +++ b/lib/WeBWorK/ContentGenerator.pm @@ -695,6 +695,7 @@ sub webwork_js_config ($c, $showMathJaxErrors = 0) { return encode_json({ webwork_url => $c->location, mathJaxBSColorSchemeUrl => getAssetURL($c->ce, 'js/MathJaxConfig/bs-color-scheme.js'), + csrf_token => $c->csrf_token, $showMathJaxErrors ? (showMathJaxErrors => true) : () }); } diff --git a/lib/WeBWorK/ContentGenerator/API.pm b/lib/WeBWorK/ContentGenerator/API.pm index ea1c6dcf34..2d48d5e7eb 100644 --- a/lib/WeBWorK/ContentGenerator/API.pm +++ b/lib/WeBWorK/ContentGenerator/API.pm @@ -48,6 +48,9 @@ sub go ($c) { return $c->renderError($c->maketext('Authentication failed. Log in again to continue.')) unless $c->authen->was_verified; + return $c->renderError($c->maketext('Invalid csrf token. Possible cross site forgery attack.')) + unless $c->param('csrf_token') eq $c->csrf_token; + writeCourseLog($c->ce, 'activity_log', $c->prepare_activity_entry) if $c->stash('courseID') && $c->ce->{courseFiles}{logs}{activity_log};