[pLog-svn] r6566 - plog/branches/lifetype-1.2/class/action/admin

[Jon Daley]

On Wed, 18 Jun 2008, mark at devel.lifetype.net wrote:
> For input validation, there is no old way or new way. Just only basic way and advanced way.
 	Yes, I understand that better after I wrote that comment.

> For advanced ways we can use validate() to perform complex validation, 
> include composite validators or complex logic validation.
>
> In this case, if the registerFieldValidator() is not suite for this 
> action, we don't need to force us to use it. It just make the action do 
> more redundent jobs.
 	In this case, shouldn't the blogId be validated in the 
constructor, and then you can do the fancier check in the validate() 
function?  Is it valid to have a non-integer blogId?  It worries me to 
have $this->_blogId set to an arbitrary string or whatever.  I'd prefer to 
see $this->_XXX not used inside the validate function, since people later 
on would assume that $this->_blogId has been validated, and in this case, 
it has not.
 	Maybe I am looking at something wrong, but I think this method 
currently is still exploitable.

>
> Modified: plog/branches/lifetype-1.2/class/action/admin/adminmainaction.class.php
> ===================================================================
> --- plog/branches/lifetype-1.2/class/action/admin/adminmainaction.class.php	2008-06-18 06:50:59 UTC (rev 6565)
> +++ plog/branches/lifetype-1.2/class/action/admin/adminmainaction.class.php	2008-06-18 06:58:02 UTC (rev 6566)
> @@ -22,7 +22,6 @@
>         function AdminMainAction( $actionInfo, $request )
>         {
>         	$this->AdminAction( $actionInfo, $request );
> -			$this->registerFieldValidator( "blogId", new IntegerValidator());
>         }
>
>         /**
> @@ -31,8 +30,9 @@
>         function validate()
>         {
> 	        // first of all, check if we have a valid blog id
> -	        $this->_blogId = $this->_request->getValue( "blogId" );
> -            if( $this->_blogId == "" || $this->_blogId < 0 ) {
> +	        $this->_blogId = $this->_request->getValue( "blogId" );
> +	        $intVal = new IntegerValidator();
> +            if( $this->_blogId == "" || !$intVal->validate( $this->_blogId ) ) {
> 	            lt_include( PLOG_CLASS_PATH."class/dao/users.class.php" );
>
> 				// check if the user really belongs to one or more blogs and if not, quit
> @@ -109,12 +109,11 @@
>         {
> 			// we don't have to worry about much more here, we can let the
> 			// $this->_nextAction action take care of everytyhing now...
> -			// If $this->_nextAction is null, we use "newPost" as default nextAction
> +			// If $this->_nextAction is null, we use "newPost" as default nextAction
> +			lt_include( PLOG_CLASS_PATH."/class/filter/htmlfilter.class.php" );
> +	        $this->_nextAction = $this->_request->getFilteredValue( "action", new HtmlFilter() );
>
> -                // TODO: validate this
> -	        $this->_nextAction = $this->_request->getValue( "action" );
> -
> -			if ( $this->_nextAction ) {
> +			if ( $this->_nextAction && AdminController::checkActionExist( $this->_nextAction ) ) {
> 				AdminController::setForwardAction( $this->_nextAction );
> 		    } else {
> 			    if( $this->userHasPermission( "new_post" ))
>
> _______________________________________________
> pLog-svn mailing list
> pLog-svn at devel.lifetype.net
> http://limedaley.com/mailman/listinfo/plog-svn
>

-- 
Jon Daley
http://jon.limedaley.com
~~
Common sense is not so common.
-- Voltaire