Potentially spam/abuse using comment form about expressionengine

[TomJaeger]

Thanks for taking the time to write all of this up! I believe there's could be a misunderstanding of how comments work.

The global setting is just for turning comments on or off on a global level. From there, each channel and role DOES have it's own comment settings.

Each channel has comment specific settings under Settings tab on a channel in the "Commenting" section - as well as specify if they require a valid EE member to be allowed to comment.

Each Role in ExpressionEngine also has comment specific settings under the "Website Access" tab in a "Commenting" section - where you can allow or deny a given role to comment.

[nep]

I think @SyntaxDreamer is saying that certain member groups should be able to post on certain channels only. There's currently no way to enforce this at submit time -- a hacker can create a form that successfully submits a comment to an entry even if there's no existing form on your website that does that.

But you can create display-side code that filters comments only to certain members (a role filter on exp:comment:entries might be useful here). And you can create an admin page that shows you comments in a certain channel but not from certain member groups. That's useful for moderating things.

Overall though, yeah, the linkage between commenting and member roles / permissions is not as complex/robust/protected as it could be.

[SyntaxDreamer]

Yes, but those settings don't stop someone from posting a comment on a channel entry, content or page they should not be posting a comment in the first place.

I just tested this on my site before posting this here on GitHub and it worked as expected. This is not some likely scenario, but something that worked in my dev install fine.

I created a ticket system in which each new support case is a new channel entry owned by the customer. Further updates then are done as comments either by the customer or staff because most are very short replies, a few words. Making a new channel entry for each reply makes no sense when those are just comments on the ticket (EE entry).

I think I was aware of this issue when I designed that ticket system. Reason I used unique URL's for each entry ID so they are not easily guessable. In that case, it's not an issue because one logged-in user can't just guess the random URL from another ticket user. But it worked. I was able to post a comment from one logged-in user, into the ticket from another user. Bad!

But yesterday I started to design a status system, it's almost finished and very nice. When an incident or schedule maintenance is created, it is a new entry in that channel. Of course, the channel is restricted to the IT person only, me.

However, then incidents have to have further updates. Think here about the Atlassian Status page, used by many companies to post incidents about issues or downtimes with their services. On each incident, you then post updates. My design is actually almost a clone, just using EE, no PHP code or add-ons.

Like I said, in the ticket design I created, it's not as critical since the URL is hidden for each logged user. But in this case, using comments for the incident updates would be a real security nightmare because the status page is public. Anyone can see the URL and segment.

So what would happen here?

The IT guy like me posts a new entry from his backend or control panel as a new entry. Visitors, customers, etc. can't post or edit any incidents as the channel has the proper permissions.

I need to follow with updates as the incident progresses. If I do this as comments (which makes absolutely sense here) since they are actually incident comments (updates), nothing stops a user from modifying the comment form on a blog article on the site and suddenly, you now have any registered person posting updates to your incident status page.

This is terrible, and I would consider it a design security issue (I think anyone would). I did not post this as a security issue because technically EE does not have any permission setting for comments, but I suspect you think the settings you mentioned do something which they don't.

You can absolutely post comments to any entry, including ones you should not.

[SyntaxDreamer]

I think @SyntaxDreamer is saying that certain member groups should be able to post on certain channels only. There's currently no way to enforce this at submit time -- a hacker can create a form that successfully submits a comment to an entry even if there's no existing form on your website that does that.

But you can create display-side code that filters comments only to certain members (a role filter on exp:comment:entries might be useful here). And you can create an admin page that shows you comments in a certain channel but not from certain member groups. That's useful for moderating things.

Overall though, yeah, the linkage between commenting and member roles / permissions is not as complex/robust/protected as it could be.

Exactly, you don't even need a comment form on the site, that is just the super easy way. You can just create a local HTML page on your computer, take a CSRF toke from the site (like the public contact form) and start posting comments to any channels that allows comments.

There is no setting because that would probably be considered a new feature. But a simple hash with a few lines of PHP would solve this. I can look into this, but I'm not very experienced with the core EE code. The part that approves comments, basically checks the hash, if it does not match, there should be an error because the comment is clearly not coming from an EE page where it was originally rendered. This would require no setting on the backend, it would just work right in the code that is executed after the comment is posted before it is saved to the database, and another code that is executed before the comment form tag is rendered to print the hash in the form as well.

I know this will work because I did this for custom code I wrote in the past on many forms and still do. If someone modifies just one letter in the form values (URI or ID), the POST will be denied since the hash does not match in the backend. This forces the user to leave the form values intact, which means he can only post to the page on which it is supposed to publish the comment.

The hash just computes the entry_id+URI. Nothing else. Both together are the hash.

In my examples above, this would prevent someone from modifying, for example, any site comment form and posting a comment to another page which they should not. Now they can only post a comment on the page on which the comment form was rendered. This actually makes total and complete sense. You should only be allowed to use a form on the page it was originally rendered.

[intoeetive]

@SyntaxDreamer thank you for laying out this so detailed.

I think the main problem here is that a spammer can potentially change entry_id in the comment form, and this way submit a comment for entry where he'd normally have no permission to comment, am I getting it right? I think URI does not matter here as that's only used for email notifications.

We do already have a way to pass protected (encoded) form parameters, so we can make entry_id one of those

[SyntaxDreamer]

@intoeetive yes, but encoding the entry_id alone is not enough. The ID's are sequential, they are predictable as they are auto-incremented from the database. Many EE users might use ID's somewhere in their visible URL segments or site design in general, which means they are exposed, and they are also exposed in other EE forms already everywhere. The ID's alone are not a security measure.

Even if that was not the case, abusers can just try all possible numbers to insert spam. This is why you need 2 values and I used the URI as an example, while you could use the action URL I am not convinced that is good for designers that need the action URL visible for development in the HTML, so it's best just to use the URI value which also matches a segment in the URL. Actually, any backend side on EE that checks the segment or URL will work.

Correct me if I'm wrong, but if the URI generated in the comment form, always matches the segment, and it's always generated by the comment form, then it can be used, otherwise a URL or segment has to be used for this second value beside the entry ID. This should be a value that is always generated before rendering the form.

You mentioned it's used for emails, but this is not actually relevant here, since we are not going to use this for any function except to create the unique hash. The idea is the URI matches the segment/url on which the comment form is generated, which means now in addition to the ID the check results in:

Is the comment data POST coming from the URL we generate the comment form on? YES
Is the comment POST going to the entry ID we want it posted? YES

If both matches, the comment is accepted.

If one of them does not, the comment is not accepted.

Since both values are generated on the backend server side, they are unique and safe, and if the user tampers with them on the front end source code, it's now irrelevant, they are just placeholders because the EE PHP backend is checking for them.

This should also be changed on each page reload to avoid fixed hashes which can be copied.

Here is a simple code I just created that could easily adapt to the EE before the comment form is generated, adding a new hidden value, and then just checking this after the comment is received. The hash here is unique, a new one each time you load the page. The hash could be used with a native hash function that EE probably and uses in other parts, and the backend most likely already has a URI similar as well.

$entry_id = '232'; // This is generated by the EE back end before rendering the EE comment tag on a site template
$URI = 'updates/software-update/just-an-example-url-segment'; // This is generated by the EE back end before rendering the EE comment tag on a site template

$dynamic_data = time(); // current timestamp for my test, use some other unique EE key to make the hash dynamic can be used here....

$raw_string = $entry_id . $URI . $dynamic_data; // We need both values, one alone is not enough

$hash_value = hash('sha256', $raw_string); // We hash both values, use any hash method. A faster option is preferable since it is not something very security sensitive

//echo "Hash Value : " . $hash_value; // When using the {exp:comment} tag in a comment form, this value will be also put as a hidden field

# The user now hits submit on the front end template, POST is data is now processed by EE

# On the EE backend that process comments we check the received value:
$received_hash = 'randomhashbecauseserchangedsomething'; // The user changed the hidden value on either URI or entry_id on the form.

// Check if the re-created hash matches the received hash
if($hash_value === $received_hash){
    echo "Success: Hashes match!"; // Process further EE code that inserts comments into the database
} else {
    echo "Error: Hashes do not match!"; // Exit all code and do not process comment, show an error on the front end
}

[intoeetive]

@SyntaxDreamer thank you for the input.

It looks to me that what is needed is a way to make sure the ID of entry cannot be guessed or accessed by other means (e.g. from another form). As said, I believe we have a way to do that in EE.

We'll work on this feature request

[SyntaxDreamer]

@intoeetive Curious how this will be achieved if the ID is visible, for example in the URL on which a channel entry form template is loaded?

Example:
https://example.com/segment/segment2/2244

Being 2244 the ID entry loaded.

A URL like that is very much possible and actually used on many EE websites.

[nep]

EE can encrypt values in a way that only it can decrypt. So we can add to core a hidden field like name="entry_id" with value="jsdhfjsdhfjkhsdkjfh" and no one could create an alternative form with a field that was decryptable to a different valid entry_id. This is different than just hashing the value, and more secure. But I'm just spitballing here. Overall, I like to share the problems I see with the EE Core team, and let them come up with the best way to solve each problem.

[SyntaxDreamer]

@nep encryption is reversible and a constant result, hashing is a one-time process if done correctly with a salt key and unique per generation (with the example I posted) which means it's only valid for that specific page request. We don't need to reverse the data (encryption) for later use, your EE installation does not need to know the ID that is passed because it already knows the ID and data before generating the form.

Encrypting the values in a form is a different scenario and is done when you are passing sensitive data. In this case, we are not actually passing any sensitive data. It is unimportant if someone knows the entry ID, or the URL, both are public anyway.

The idea is not to hide the ID's, which is why I'm confused from @intoeetive replies. What we want is just data integrity checking. Not hide or encrypt data.

If we encode or encrypt the ID, nothing stops someone from taking that encoded string and still using it on a local HTML created form and still commenting.

The hash here is just to avoid tampering with the data, not to protect it. Because the code is open source, even by reading this, even if someone knows, ahh, that hash is composed by the URI and entry ID. The way the hash would be created on the server side would probably use something unique from that installation (salting or key) just like it hashes the password, it's not supposed to be reversible.

When someone logs in, EE does not check the password because it does not store passwords in the database, it just takes the input, hashes it, and compares it to the saved hash. Same logic here. We compare if the hash on the FORM once posted matches the one on the server side. This is not actually storing data like passwords, and it's not something very critical, it can just be a basic quick hash for performance reasons and not even saved anywhere. We only use the hash as an integrity check, not to store sensitive data.

This is why I mentioned the URI as well. What we want is passing the authentication from the user who is posting on the form to Expression Engine or the back end server. Frontends can be tampered with and are not secure, server side is. By not allowing a comment to be posted from the page on which it was generated, you can now benefit from the Expression Engine permission security settings.

You can now set the permissions for the user who has access to the entry_id form template based only on the user that will interact with said form. Or you can set the parameter in the channel entry by user ID, logged-in user, group, etc. Or just surround the form tags with the proper permissions. The comment form now benefits from all those permissions automatically, just like the channel entries do.

We are not trying to prevent someone from messing with the entry_id. That is already public knowledge. The hashing example I propose is just to make an integrity check from where the comment is coming from. Encoding the entry ID, alone I can still register on the site, take the encoded value and create my own comment form and start posting. Now, even if the admin disallows that user access to this restricted template, that would have no effect because the malicious user can still post even without actually accessing the template or the page directly.

With the integrity hash or a similar method native to EE, this would not work. If someone tries to use another comment form, or create it in HTML from another part of your site, the comment would be rejected, this means the person can only submit the comment form from the page it was originally served, now an admin can block the user as well, by setting the permission blocking his access to that template.

I tried to explain as much as possible, but I think some are still confused about this, so I will try again:

You create a page called Page-1 with a channel entry form that has a comment form. Your entries allow comments here.

Page-1 allows users in Group_A to post comments here.

You have another entry form on Page-2 that also allows comments. This Page-2 might be entirely different and serve an entirely different type of content as opposed to Page-1.

Page-2 can only be accessed by users in Group_B. You can set this on the EE control panel or in the template, it is not relevant....just like most people set permissions on their sites already for logged-in users. The idea is Page-2 is restricted to only a set group of users, like your staff, customers, whatever.

Both Group_A and Group_B are allowed to post comments on their permission group set from the control panel admin. Both Page-1 and Page-2 have comments enabled on their channel entries permissions on the admin control panel.

You restricted Page-2 for access only by members on Group_B . It makes sense that only users from Group_2 can comment here, right?

Now, even while Page-2 is restricted to Group_B, users only. Users on Group_A can still comment on your private Page-2 even without having permissions to access this page at all. It's a member's only page, or they have are denied access if they try to load it. But as today, all members who have commenting enabled would be able to post comments, even to private pages or templates that are restricted on permissions.

By checking the POST is coming only from Page-2, you are stopping a malicious user from posting comments on Page-2 which is the default behavior most people probably expect. The only way to post a comment on Page-2 is now only from the form loaded on Page-2 which means without access you can't create comments.

[nep]

What a great description of a way to handle this. Thank you, @SyntaxDreamer . I look forward to seeing what gets implemented.

[intoeetive]

@SyntaxDreamer sorry about not getting it exactly right - I seem to have the kind of "twitter mental disease" - it's hard for me to follow long texts. I am trying to get the essence, and while I appreciate you describing this in 19 paragraphs, it does not help me personally.

So I think I got the problem right: the users are able to submit comment form to another entry, which normally they can't see

On my initial understanding, I thought the solution you propose is to encrypt entry_id

From your last comment, I see that the solution you really propose is adding integrity check for some of the form data that is not supposed to be changed by user.

Am I getting it right this time?

Please note that we're still in discussion phase here and will also have internal discussion, so I'm just making sure I have understood the essence of your request correctly.

(note to self: we have unrelated request for form integrity check in terms of list of fields submitted, so might combine the two)

[SyntaxDreamer]

@intoeetive Yes, that is exactly correct. You can submit comments to an entry/page you can't access by using another form on the site. An admin might restrict the template or set the permissions for a channel form, but this has no effect on the comment form.

Yes, the hash in this case is not to hide or protect the data, just for integrity reason to force the comment to be on the same page as the form code was rendered. That way, you can't use another comment form on the site to submit comments to all channel entries on a site, even those without access.

[robinsowell]

Question- would adding a member_id and a member_role_id parameter to the comment form solve the issue?

[robinsowell]

Though thinking on it, if they're faking the form, they could just not include it. So... that would have to be a channel specific setting. Which- would I think solve it?

[SyntaxDreamer]

Though thinking on it, if they're faking the form, they could just not include it. So... that would have to be a channel specific setting. Which- would I think solve it?

From the user side you can tamper with any form or its data values, which includes any user or channel ID numbers directly on the page source code. The only fix is checking a parameter back on the server side that has to match with was originally rendered on the form.

It has to be a unique value that changes every time the form is generated and has to match some checksum on the server side.