Skip to content

Conversation

@jesusbagpuss
Copy link
Collaborator

Prototype is not being developed/released any more, but there is a fix for the CVE detailed in: prototypejs/prototype#349

I don't think that this CVE is 'dangerous' in relation to the regular EPrints codebase, but it does get flagged by scanning services.
The version string 1.7.3.1-eprints has been invented for our purposes. It is used by prototype when constructing XMLHttpRequest headers.

NB There are also other things that were committed to the prototype master branch since the 1.7.3 release, but I don't think it's worth doing anything with these, as EPrints will not rely on Prototype in the future.

Prototype isn't maintained, but the CVE can be resolved.
Taken from:
prototypejs/prototype#349
Change Version string
@drn05r
Copy link
Contributor

drn05r commented Feb 22, 2025

It should be noted that EPrints 3.5 will remove Prototype JS, as it is now possible to carry out all its EPrints actions that requires it for using generic JavaScript.

@drn05r drn05r self-requested a review February 22, 2025 18:20
Copy link
Contributor

@drn05r drn05r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see how this change should also incorporate HTML tags that do not have matched quotes marks. However, I am a little puzzled why the regex is not:

this.replace(/<\w+(\s+("[^"]"|'[^']'|[^>'"])+)?\s*("[^">]*|'[^'>]*)?(/)?>|</\w+>/gi, '');

As otherwise it looks like it would match <h1 style="color: red> but not <h1 style='color: red> .

@jesusbagpuss
Copy link
Collaborator Author

I agree that the * is a more complete fix.
Both versions resolve the ReDOS CVE as tested with this: OpenMage/magento-lts#3003 (review), but I think updating the PR is correct - I'll do that now.

Copy link
Contributor

@drn05r drn05r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regexp looks good to me now.

@drn05r drn05r merged commit 094711b into eprints:master Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants