Requests: No Length Upper Boundary for Comment Messages

[Samk13]

Package version (if known): v12rc2 / latest

Describe the bug

The comment feature in the Requests allows users to send comments with no limit on length, posing a security risk such as denial of service attacks or system crashes due to excessively long messages.

Steps to Reproduce

1. Go to the Requests package.
2. Compose a new comment.
3. Keep adding text without any restriction.
4. Observe that there is no limit enforced, allowing potentially dangerous message lengths.

Expected behavior

The system should enforce a reasonable limit on the length of comments to prevent abuse and ensure stability.

Screenshots (if applicable)

Links:

https://github.com/fenekku/invenio-requests/blob/master/invenio_requests/customizations/event_types.py#L145

https://github.com/inveniosoftware/invenio-requests/blob/82dbf2885c8e777caa1c5163971ab5c31aca5398/invenio_requests/records/jsonschemas/requests/definitions-v1.0.0.json#L12

https://github.com/inveniosoftware/invenio-requests/blob/master/invenio_requests/services/events/service.py

[Samk13]

Questions:

• What is the acceptable max length for comments?

[kpsherva]

How big do you consider this to be a security risk since the commenting is restricted only to users who have access to particular request? In most of the instances the users who are authenticated in the system are verified and part of the institution who runs the instance, therefore the malicious intent is quite limited. Please let me know if you are aware of other cases - otherwise I wouldn't consider this as a v12 release blocker

[Samk13]

Thanks for the comment @kpsherva . While logged-in users are generally trusted, risks include:

• ORCID Logins: Easy to obtain, not highly secure, can be misused.
• Insider Threats: Verified users can still be malicious.
• Human Error: Unintentional long inputs can cause instability.
• Best Practices: Limiting input length is standard for security and stability. Of course, it's not a blocker, but it's a low-risk improvement that enhances security and stability, so why not include it in V12?

[Samk13]

After a DM on Discord, it’s decided that v12 will focus on critical fixes due to resource constraints for testing. This could be included in v12.1 if the scope is clear and resources are available.

[github-actions[bot]]

This issue was automatically marked as stale.

[github-actions[bot]]

This issue was automatically marked as stale.