Security vulnerabilities

[drgrice1]

These are security vulnerabilities that @Alex-Jordan identified using Claude.

1. There was a possible SQL injection vulnerability in the getDBextras method in lib/WeBWorK/Utils/ListingDB.pm. The $path and $filename could potentially come from user provided parameters and were interpolated directly into a double-quoted SQL string with no placeholders.

2. There was a potential path traversal vulnerability in lib/WebworkWebservice/RenderProblem.pm that made it possible to read a file outside of the course templates directory. A user with permission to use the render_rpc end point could potentially read the /etc/passwd by using a sourceFilePath such as ../../../../../etc/passwd. Although, even if that is done I could not find a way to actually see the file's contents (other than a couple of characters when rendering of the file as a PG problem fails). By using a path such as ../../otherCourse/templates/otherCourseProblem.pg an instructor could render a problem from another course, but I don't consider that a very high security vulnerability.

3. An instructor could engineer a request (by modifying parameters in the browser using the developer tools or using a script) to the PG problem editor to add a problem using the add_problem action and pass a sourceFilePath such ../../../../../etc/passwd and a new problem would be added to the set with that file path. If the instructor then opens the problem in the set, the entire contents of the file will be revealed in the errors when the file fails to render as a PG problem. This could also be used to render a problem from another course's templates directory, and it would even work in the assignment. The possibiity of viewing a file such as /etc/passwd file is a high security vulnerability, but rendering a problem file from another course, probably not so much.

4. Restrict the instructor_rpc endpoint to POST requests only. The exploit that Claude describes is that an attacker could emails an instructor a URL like such as https://server.edu/webwork2/instructor_rpc?rpc_command=putUserProblem&courseID=Math101&user_id=someStudent&...&status=1. Clicking it sends the session cookie, and the change (grade/status edit, user add/delete, etc.) executes. Disabling GET requests prevents that. Claude also recommends adding a CSRF token, but I don't see that as necessary. It is contrary to the API like design of the Webwork web service, and is not a usual protection that is put into place by other systems for this sort of thing either. Furthermore, this would not actually add security. It would mean that in order for webwork2 itself to use the end point, it would have to make the CSRF token available in the DOM for the JavaScript to be able to attach to any request. That is the same basic security vulnerability that could be exploited in the same way that the session key could be exploited when using $session_management_via = 'key'.

5. Use Math::Random::Secure::irand to generate the session key instead of srand and rand so the the session key is cryptpographically secure. The protects against the possibility of an attacker triggering many logins (including an unauthenticated guest login, which also calls create_session) and harvesting a large sample of session keys generated by the same worker process, and attempting a Mersenne-Twister state-recovery techniques to predict keys issued to other users on that worker. Although, this may only be a vulnerability if guest logins are enabled for a course. Note that Claude also said to fix the identical code in WebworkSOAP.pm, and I did, but I plan to delete WebworkSOAP.pm for the next release. I would be willing to do that for this release if no one is opposed! I have it all set up in a local branch.

6. There was a possibility of an LDAP filter injection when LDAP authentication is enabled. A username could be altered with a username such as admin)(|(uid=* which would change the DN the filter would resolve to. This allows an attacker to redirect which account's DN gets targeted, allowing for an account-confusion attack. The fix is to use the Net::LDAP::Util::escape_filter_value method on the $uid before it is passed to the $ldap->search call. Note that the Net::LDAP::Util module is part of with the Net::LDAP package, and so this is not a new dependency.

7. Both LTI authentication modules save the first and last name of a user that authenticates via LTI to the database. Those are things that in some cases a user can set on their own (depending on if the institution chooses to allow that), and a user could set one of those to include a <script> tag with malicious code. The first and last name are displayed in several templates unescaped, and so a user could potential exploit this to perform an XSS attack on their instructor. I tested this with my mock LMS using 1.3 authentication, and the script I set for the user's last name was executed on the user detail page (the sets assigned to user page that opens when you click on the "Assigned Sets" column in the "Accounts Manager"). Testing with Moodle I observed that Moodle immediately strips off the <script> tag and any special characters, and so that never makes it to webwork. Testing this with Canvas I observed that Canvas does not, and the script executes when opening the user detail page for the user.
   In any case Claude's suggestion to escape these in the templates isn't going to work as that would break the things that are being done in those templates. So instead this just strips any ampersands, less thans, greater thans, or quotes from the first and last name when they are received during LTI authentication. Single quotes are left since they will not be a problem and are common in names.

Note that I did not use Claude to fix them, although in some cases the fixes are just what Claude recommended. The coding wasn't that hard, and Claude's recommendations that @Alex-Jordan sent me were not correct. Furthermore, I doubt that Claude would have been able to do the testing (particularly with LTI authentication) that I did.

There were 2 other vulnerabilities that were listed that were not fixed.

Both relate to symbolic links in a course templates directory. The first is basically allowing any symbolic link to be extracted in the file manager when a tarball is extracted. The second is that the PGProblemEditor can read files linked to by a symbolic link in the course templates directory. The second is really only a vulnerability in combination with the first, assuming that the only links in the course templates directory are the ones that the webwork system administrator wants to be there (such as Library, Contrib, Student_Orientation, or other local libraries), and permissions are correctly set for the files in the directories linked to. This is because currently extracting a tarball (and maybe a zip archive as well which Claude didn't notice) that contains a symbolic link to somewhere else on the system is the only way that such a link can be created by an instructor. So to fix both issues we will have to stop allowing symbolic links in extracted archives. This would be a change to what we have allowed to this point, but is probably what we are going to need to do. So the only allowed symbolic links in a course templates directory would be links created by the webwork2 server administrator.

[Alex-Jordan]

I reviewed this by reading each item in your summary and finding the corresponding code changes and reading those. (That is, I did not directly test the vulnerabilities.) It all looks good.

As to the SOAP module, I defer to whatever you would want to do.