Blog Comment Form sharing invalid data across all users

Topics: Troubleshooting
Oct 3, 2014 at 4:11 PM
We are running into some pretty strange issues with our Orchard installation. Our blog display page (Parts.CommentForm-BlogPost.cshtml) has the comment form shown at the bottom of the page and we are seeing these issues with it:

Here is an example page: http://wintellect.com/blogs/jlane/securing-node.js-applications-with-oauth2-and-azure

• The blog comment form is pre-populated with SPAM garbage when the blog display page is loaded before the user ever types anything and it is the same data across all anonymous users and in all browsers.
• If I enter invalid data into the comment form and submit, I can trace into Comment/Create and see that the Model is NOT valid and I can see the Html.ValidationSummary defined in the Parts.CommentForm-BlogPost.cshtml, but no validation is ever surfaced to the UI or the user.
• This invalid data that I entered is still saved somewhere (TempData?) and then all anonymous users hitting the site now get the same invalid data that I typed shown pre-populated into the comment form. To see this in action, visit: http://wintellect.com/blogs/jlane/securing-node.js-applications-with-oauth2-and-azure in one browser. You should see garbage in the comment form. Try to submit without filling in the CAPTCHA so it is invalid. Go to a different browser and hit the link. You will see the invalid data you typed in the first browser pre-populated in the 2nd browser!

Why is the data shared across all anonymous users? Especially invalid data entry?

Why is the validation not showing in the UI?

Any assistance in narrowing down the root cause of this issue is greatly appreciated.
Oct 3, 2014 at 10:16 PM
Edited Oct 3, 2014 at 10:23 PM
Hi!
  • First of all I can't recreate your problem with the pre-populated SPAM on your site. It's fine for me.
  • I think the error messages for the comments are displayed in the "message-zone" (i am not 100% sure, but its on my site that way). So please check your theme, if there is a message zone defined.
edit: Ok, after a refresh I can see the pre-populated SPAM :/

edit2: I think it has something to do with the cache. Because if I refresh the page with "Shift + F5" the spam is not here, if I refresh it with just F5 voila SPAM is there.
Coordinator
Oct 3, 2014 at 11:18 PM
I worked with r2musings offline and I could repro the issue, this is an output cache one. He can repro it locally so we'll debug what is happening.

And btw the messages not appearing was because their theme was not displaying the Messages zone.
Jul 17, 2015 at 3:33 AM
Edited Jul 17, 2015 at 3:42 AM
Anything further to add to this? We are seeing the same issue with a website of ours. Spam comments displaying prepopulated in the comments field.
Jul 22, 2015 at 3:58 PM
I have nothing to add. My company abandoned Orchard in November and moved to WordPress. Hope you can get it working.
Coordinator
Jul 22, 2015 at 10:29 PM
My bad then, I must have lost track.

Michael, would you mind sharing your version number and some repro steps, even from your website?
And also the infrastructure you are using, like any reverse proxy, or cache settings.
Jul 22, 2015 at 11:19 PM
I'm running 1.8.1 at present in a multi-tenant environment. I ended up working around this problem by disabling output cache on this tenant. While not ideal, this tenant is a low traffic site so the impact isn't too bad.

I'm not certain what was happening, but I believe some kind of spam bot was continually posting details through the form. Before disabling the cache I observed that when I evicted the page from cache, it would be re-cached almost immediately, then upon loading the page through a different browser that wasn't logged new spam details would be repopulated in the form.

I haven't had time to investigate or look into the behaviour of the cache module in detail, however I plan to upgrade the Orchard instance to 1.9.1 very soon, and I noticed there is a patch in there to prevent the output cache from caching responses that contain notification messages. I was initially thinking that this might resolve the problem because then if a spam bot posts a comment, the comment being rejected would return a notification message so the page wouldn't get cached. However I have a few concerns about this line of thought (but haven't investigated)...
  1. I'm using the javascript anti spam feature, so if this treats the comment as spam (which it is because I'm not actually getting these spam comments stored in my system) I'm not sure that this feature will actually raise a notification message. If it just silently fails and returns the page with the form (containing the spammy form data) is there a possibility there for the response to still get cached?
  2. In general, should we ever be attempting to cache the returned result of a POST request? I figure when a POST is successful there is no problem, because a redirect response of some kind is issued. But in the case of a validation error, I suspect we're returning the view model back directly (along with the client's spammy form data) and this is being cached. Would it be safer to check somehow (in the output cache filter) if this response is from a POST request, rather (or in addition to) checking that if the response has any notifications?
Coordinator
Jul 23, 2015 at 11:28 PM
Not caching if a notification is displayed or when a POST is issued is already supported. There might have been some fix on the notification though, I remember working on that. So yes, please try on 1.9.1.

The javascript feature should not impact, but we never know, I could imagine it also triggers a notification.
Aug 3, 2015 at 2:01 AM
I've now upgraded our environment to 1.9.1 and retested and this problem is still occurring.

I can reproduce this now...
  1. Install blog, comments, java script anti spam and caching modules
  2. Associate the javascript anti spam part to the comment form
  3. Enable output caching
  4. Open two different browsers, one logged into the dashboard in the cache configuration -> statistics page. One anonymous and go to a blog article that has the comments form at the bottom
  5. After the page blog article page is loaded if you refresh the statistics on the other browser you should see this page listed in the cache stats.
  6. Now evict this page from the cache
  7. Now fill in some valid form data in the anonymous browser, then use the browser's object inspector to remove the Iamhuman hidden field from the form
  8. Now submit the form - The javascript anti spam check will fail, and the anonymous browser is reloaded
At this point no notification message is raised, and it appears the output cache proceeds to cache the result (including the pre-filled form data)

If you open another anonymous browser and go to the blog page, you'll now see the pre-populated form data.

So it appears that the returned display result from the post request is being cached. This seems a little scary doesn't it?
Aug 3, 2015 at 2:57 AM
With further investigation I believe this may be a problem in the comments controller and how it handles invalid comment data.

Instead of just returning the View(model) from CommentController.Create when the model state is invalid, the action adds the comment form editor to TempData and redirects to the blog post. The comment part view then extracts this data from the temp data. Because this just looks like a normal get request the output cache proceeds to cache the display result (including the spammy comment data).

Why is the comment controller doing this this way? Doesn't seem right.
Aug 3, 2015 at 3:12 AM
In the OutputCacheFilter.cs class in the output cache module there is a method "ResponseIsCacheable".

In my test environment I first tried adding an additional check to ensure the response isn't cached if the model state is not valid. This didn't work since the model state is actually valid - it just has the dodgy form data brought in through the TempData variable by the comment part.

In the end I worked around the problem by adding a check to the outputcachefilter specifically for the existence of this comment form in the TempData. This resolves the issue but really feels like a hack and I believe the fix should be to change the way in which the comment controller and comment part view are handling invalid comments. Here's both checks just for reference...
            // Don't cache if the response's model state is invalid
            if (!filterContext.Controller.ViewData.ModelState.IsValid) {
                Logger.Debug("Response for item '{0}' will not be cached because the model state is invalid.", _cacheKey);
                return false;
            }

            // Don't cache if the temp data contains invalid comments
            if (filterContext.Controller.TempData.ContainsKey("Comments.InvalidCommentEditorShape")) {
                Logger.Debug("Response for item '{0}' will not be cached because it contains comment errors.", _cacheKey);
                return false;
            }
Would it be good to leave the model state check in (even though it didn't resolve my problem) - as a safeguard we probably never want to be caching results that contain model errors?