[pLog-svn] r5918 - plog/branches/lifetype-1.2/class/security

oscar at devel.lifetype.net oscar at devel.lifetype.net
Fri Sep 7 17:38:01 EDT 2007


Author: oscar
Date: 2007-09-07 17:38:00 -0400 (Fri, 07 Sep 2007)
New Revision: 5918

Modified:
   plog/branches/lifetype-1.2/class/security/bayesianfilter.class.php
Log:
This should solve issues http://bugs.lifetype.net/view.php?id=1386 ("Spammers are able to post comments even if comments are disabled for a particular post") and http://bugs.lifetype.net/view.php?id=1387 ("comments with article_id = 0 created by some spam bots")

The problem here was that since the bayesian filter is run *before* any application logic is run, it should also check things like whether comments are enabled or not and if the article is found at all or not, even though this same checks are applied later on in the AddCommentAction class. The articleId parameter was taken as is from the request, without performing any check other than checking if it is an integer, so this caused some comments to point to an article with an id of '0' because we did not check if the article really existed before saving the spam comment. And the same applies to the other situation, with the toggle for enabling and disabling comments.

The solution was to add some additional logic to the BayesianFilter filter class and perform these checks, that does indeed duplicate some of the logic found later in the process flow but I did not find a more elegant solution for this (at least not without a redesign of the whole filter architecture anyway)

Modified: plog/branches/lifetype-1.2/class/security/bayesianfilter.class.php
===================================================================
--- plog/branches/lifetype-1.2/class/security/bayesianfilter.class.php	2007-09-07 10:12:04 UTC (rev 5917)
+++ plog/branches/lifetype-1.2/class/security/bayesianfilter.class.php	2007-09-07 21:38:00 UTC (rev 5918)
@@ -62,7 +62,8 @@
                 return $result;
             }
 			
-            lt_include( PLOG_CLASS_PATH."class/dao/articlecomments.class.php" );			
+            lt_include( PLOG_CLASS_PATH."class/dao/articlecomments.class.php" );
+            lt_include( PLOG_CLASS_PATH."class/dao/articles.class.php" );
             
             // if it's a trackback, the data is in another place...
             $parentId = "";
@@ -86,6 +87,32 @@
                 $articleId = $request->getValue( "articleId" );
                 $parentId  = $request->getValue( "parentId" );          
             }
+
+			// the two checks below are duplicating some of the code in AddCommentAction
+			// and definitely belong to the business logic rather than to the bayesian filter
+			// logic, but the problem here is that the filter is run *before* these checks are performed
+			// and in some situations, we may end up adding comments for an article that has commenting
+			// disabled or for an article that does not even exist
+
+			// does the article even exist?
+            $articles = new Articles();
+            $article  = $articles->getBlogArticle( $articleId, $blogInfo->getId());
+            if(!$article) {
+				// if the article to which the articleId parameter refers to doesn't exist, there really
+				// is no need to process the whole comments even if it's spam, the request will not be
+				// processed by AddCommentAction for this very same reason
+                $result = new PipelineResult();
+                return $result;	
+			}
+			
+			// and if it does, are comments enabled for it anyway?
+			$blogSettings = $blogInfo->getSettings();
+            if( $article->getCommentsEnabled() == false || $blogSettings->getValue ( "comments_enabled" ) == false ) {
+				// we let this request pass through although it may be spam, since it will be blocked
+				// later on by AddCommentAction because comments aren't enabled	
+                $result = new PipelineResult();
+                return $result;	
+			}
             
             if( $parentId == "" )
                 $parentId = 0;
@@ -94,7 +121,7 @@
             
             if ($spamicity >= $config->getValue("bayesian_filter_spam_probability_treshold"))
             {
-                    // need this to get the locale
+                // need this to get the locale
                 $plr = $this->getPipelineRequest();
                 $bi = $plr->getBlogInfo();
                 $locale = $bi->getLocale();



More information about the pLog-svn mailing list