search

verbb / comments

A Craft CMS plugin for managing comments directly within the CMS.
Other
137 stars 33 forks source link

Possible to trash other user's comments by simply changing commentId? #294

Closed codyjames closed 7 months ago

codyjames commented 7 months ago

Describe the bug

We received a bug bounty report today showing us that folks are able to trash other users' comments by simply changing the commentId. Is there anything in place that is preventing this? Looking through the code I'm not immediately seeing anything that would prevent this.

Steps to reproduce

  1. Create a comment.
  2. View the source of another user's comment to find its ID.
  3. Swap the ID of your comment with the other user's comment in the Delete/Trash form.
  4. Submit the form.
  5. The other user's comment will be deleted.

Craft CMS version

3.9.6

Plugin version

1.9.6

Multi-site?

No response

Additional context

No response

codyjames commented 7 months ago

I think this may be due to validation being turned off for this specific action.

https://github.com/verbb/comments/blob/f22fc5b64f9c006750df86d3b3f4b1dc16038374/src/controllers/CommentsController.php#L326

It seems like most of your permissions/security checks are happening in beforeValidate. Perhaps that gets skipped if saveElement has the second argument set to false?

engram-design commented 7 months ago

Good call, purely an oversight here, where there's no distinction between save events and their action.

Fixed in 1.9.7

codyjames commented 7 months ago

@engram-design Thanks for fixing this up so quickly! We really appreciate it!