[geeklog-devtalk] geeklog-devel digest, Vol 1 #281 - 5 msgs

[geeklog-devel-request at lists.geeklog.net]

Send geeklog-devel mailing list submissions to
	geeklog-devel at lists.geeklog.net

To subscribe or unsubscribe via the World Wide Web, visit
	http://lists.geeklog.net/listinfo/geeklog-devel
or, via email, send a message with subject or body 'help' to
	geeklog-devel-request at lists.geeklog.net

You can reach the person managing the list at
	geeklog-devel-admin at lists.geeklog.net

When replying, please edit your Subject line so it is more specific
than "Re: Contents of geeklog-devel digest..."


Today's Topics:

   1. How to use COM_applyFilter (Dirk Haun)
   2. Re: How to use COM_applyFilter (Blaine Lang)
   3. Re: How to use COM_applyFilter (Dirk Haun)
   4. Re: How to use COM_applyFilter (Blaine Lang)
   5. Re: [geeklog-users] An SQL error has occured (Tony Bibbs)

--__--__--

Message: 1
From: "Dirk Haun" <dirk at haun-online.de>
To: <geeklog-devel at lists.geeklog.net>
Date: Thu, 26 Feb 2004 19:11:51 +0100
Organization: Terra Software Systems
Subject: [geeklog-devel] How to use COM_applyFilter
Reply-To: geeklog-devel at lists.geeklog.net

I wrote this little piece to explain the use of COM_applyFilter. It's
mainly aimed at authors of plugins and other add-ons, but most of it
applies to new code that's supposed to go into Geeklog as well. Comments
welcome (as well as suggestions on where to post it so that it can easily
be found ...).

bye, Dirk

--- snip ---

Abstract: Geeklog 1.3.9 introduced a new function, COM_applyFilter, that
is used to filter parameters passed in HTTP GET and POST requests. It is
strongly suggested that plugins and other add-ons make use of this function.
This post explains how to use COM_applyFilter and also provides additional
information on how to make your scripts more secure.

Whenever parameters are passed in an HTTP GET request (usually in a URL
of the form script.php?parameter=value) or an HTTP POST request (usually
from an input field in a form, e.g. <input name="parameter" value="value">)
there is a potential risk that these parameters are manipulated. With GET
requests, it is easy to edit the URL and manipulated POST requests can be
sent through manipulated forms or by using tools like netcat.

It is therefore important not to trust these parameters too much!

The COM_applyFilter function was designed to clear parameters from the
most commonly used injection attempts (both SQL and JavaScript injections).
So, to strip any potentially malicious content from parameters, use
COM_applyFilter as follows:

    $myvalue = COM_applyFilter ($HTTP_POST_VARS['myparameter']);

Or, in case, of a parameter that is supposed to be numeric:

    $myvalue = COM_applyFilter ($HTTP_POST_VARS['myparameter'], true);

Your script should be prepared to handle the case that $myparameter is empty
(or 0, for numerical parameters) after the call to COM_applyFilter. This will
usually be the case when content was stripped from the parameter (unless it
was empty / zero to begin with). Whether your script aborts in those cases
or continues with default values instead of the empty / zeroed parameter,
is up to you. Both may make sense, depending on the circumstances.

As can be seen in the examples above, it is recommended NOT to rely on
register_globals being "on" (although Geeklog still requires this) but
to use the global $HTTP_GET_VARS and $HTTP_POST_VARS arrays instead (don't
forget to declare them as "global" when you're using them inside a function).
The $_GET, $_POST, and $_REQUEST arrays could be used instead of
$HTTP_GET_VARS
and $HTTP_POST_VARS, but since they were only introduced in PHP 4.2.0,
you may 
restrict the audience for your plugin / add-on somewhat, as quite a few
Geeklog installs out there are still running on older versions of PHP.

If possible, you should NOT follow Geeklog's example of testing whether a
parameter is set in the $HTTP_GET_VARS or $HTTP_POST_VARS array. Instead,
write your code such that at any moment you know exactly where your parameters
would be in case of proper execution of the script. So if you know that at
a specific point in your script, parameters can only be in the $HTTP_GET_VARS
array (because you are expecting to be called through an HTTP GET request),
don't bother checking the $HTTP_POST_VARS array (instead, simply ignore it).
Geeklog's core code contains a few bad examples where at specific points in
a script it is not clear whether we came there through a GET or a POST
request and thus have to test both for the proper parameters. Depending on
the situation, it may make things easier for an attacker and the code is in
general much harder to maintain. Don't repeat that mistake.

Please note that you can NOT use COM_applyFilter on any sort of "free-
form" content, such as the text of a story or things like a user's full
name, since the function would strip out many special characters (such as
quotes) and make the content illegible and / or useless. Instead, you
should do something like this:

    $mytext = COM_stripslashes ($HTTP_POST_VARS['mytext']);
    // do something with it
    $mytext = addslashes ($mytext);
    DB_save ($_TABLES['mytable'], "mytext", '$mytext');

The COM_stripslashes function will strip any slashes that may have been added
during the POST operation, if the PHP option magic_quotes_qpc is "on" (and
leaves the text untouched, if it is off), thus ensuring that you get the text
back exactly as it was entered by the user. You can then process the text
as needed by your plugin / add-on.

Before you store the text in the database, you should call addslashes on it
to ensure that any special characters are properly escaped. This will NOT
add slashes to the content in the database, it will only ensure that the
text is properly stored (and in case it contains any SQL injection attempts,
those would be stored as text, too, instead of being executed as part of
the save operation).

Actually, it may be a good idea to apply addslashes on ALL parameters that
go into the database, even if they have been passed through COM_applyFilter
before, just in case.

On a side note, if you need to identify the current user, you should NEVER
rely on the user's id passed through GET or POST requests (e.g. by embedding
it in a form and reading it back when the form was submitted). Instead,
ALWAYS use the global variable $_USER['uid']. This variable may be empty
or contain 1, which indicates an anonymous user, i.e. a user that is not
logged in. So you should use something like

    if (!empty ($_USER['uid']) && ($_USER['uid'] > 1)) {
        // this is a logged-in user
    } else {
        // this is an anonymous user
    }

To summarize:

- use COM_applyFilter on any parameters passed through an HTTP GET or POST
  request
- add "true" to the call when the parameter is supposed to be numeric
- be prepared for the parameter to be empty or zero afterwards
- don't rely on register_globals - use $HTTP_POST_VARS and $HTTP_GETS_VARS
  instead
- write your script such that you know whether your parameters are in
  $HTTP_POST_VARS or $HTTP_GETS_VARS
- for "free-form" content, don't use COM_applyFilter but be careful to filter
  it otherwise and apply addslashes before storing it in the database
- always rely on $_USER['uid'] to identify a user


-- 
http://www.geeklog.net/
http://geeklog.info/


--__--__--

Message: 2
From: "Blaine Lang" <geeklog at langfamily.ca>
To: <geeklog-devel at lists.geeklog.net>
Subject: Re: [geeklog-devel] How to use COM_applyFilter
Date: Thu, 26 Feb 2004 14:01:53 -0500
Reply-To: geeklog-devel at lists.geeklog.net

Dirk,

This is a good discussion and one we spent a lot of time in our code
addressing. I have been using your function and variations of it now for a
couple months. I think it's a great addition to the common functions.

I had a reason (can't recall the details now) where I added the option to
"not return 0" - it would return '' instead. The default was return for a
numeric test and the parameter being tested turned out to not be numeric.
 COM_ApplyFilter( $parameter, $isnumeric = false ,$returnzero=true)

The other purpose I had was a stronger version that only allowed "letters
and numbers"
     $p = preg_replace('/^[^(0-9a-zA-Z)]+/','', $parameter );


Plugin developer either not support versions prior to 1.3.9 or need to add
their own plugin version.

I often use the same variable name for GET and POST for something like "what
operation" is being performed. Sometimes you have links which trigger an
operation but in the same application there may be a form. I have been using
$op to indicate the operation (add/delete/edit etc ...) and use code like
this:

if (isset($HTTP_POST_VARS['op']) ) {
    $op = clubApplyFilter($HTTP_POST_VARS['op']);
} elseif (isset($HTTP_GET_VARS['op']) ) {
    $op = clubApplyFilter($HTTP_GET_VARS['op']);
} else {
    $op = '';
}

I first check the POST for a request and then the GET string. This is a
repeatitive piece of code as I may have other common used variables like
recid, mode, page, sortoption etc. Example: Page navigation needs to pass
page# and Sort options in the URL string but I also need to pass these
variable in forms if I want to return the user to same view.

I have been thinking, it would be nice to be have a common function that
would filter all the POST and GET vars. It is possible to walk the arrays
and maybe send it a list of variables to clean  - using array_walk().

Cheers,
Blaine


--__--__--

Message: 3
From: "Dirk Haun" <dirk at haun-online.de>
To: <geeklog-devel at lists.geeklog.net>
Subject: Re: [geeklog-devel] How to use COM_applyFilter
Date: Thu, 26 Feb 2004 23:16:38 +0100
Organization: Terra Software Systems
Reply-To: geeklog-devel at lists.geeklog.net

Blaine,


>The other purpose I had was a stronger version that only allowed "letters

>and numbers"

>     $p = preg_replace('/^[^(0-9a-zA-Z)]+/','', $parameter );


That may make sense. We should review some of the "freedoms" that Geeklog
is giving users for 1.3.10 anyway. E.g. user names (we've discussed this
one before, I remember) or topic IDs.



>if (isset($HTTP_POST_VARS['op']) ) {

>    $op = clubApplyFilter($HTTP_POST_VARS['op']);

>} elseif (isset($HTTP_GET_VARS['op']) ) {

>    $op = clubApplyFilter($HTTP_GET_VARS['op']);

>} else {

>    $op = '';

>}


I was actually discouraging the use of this in my writings. Although it
probably makes sense to do this for one parameter (the mode of operation,
as in your example) and then, depending on that mode parameter, get the
other parameters ONLY from the array they are supposed to be in. I.e.
something like

    if ($op == 'submit') {
        // 'submit' will always indicate a POST request
        $something = COM_applyFilter ($HTTP_POST_VARS['something']);
    } else if ($op == 'new') {
        $whatever = COM_applyFilter ($HTTP_GET_VARS['whatever']);
    }



>I have been thinking, it would be nice to be have a common function that

>would filter all the POST and GET vars. It is possible to walk the arrays

>and maybe send it a list of variables to clean  - using array_walk().


array_walk sounds useful, but I'm not sure how you would derive a common
function from it. How would it know which parameters are supposed to be
numeric?

bye, Dirk


-- 
http://www.haun-online.de/
http://geeklog.info/


--__--__--

Message: 4
From: "Blaine Lang" <geeklog at langfamily.ca>
To: <geeklog-devel at lists.geeklog.net>
Subject: Re: [geeklog-devel] How to use COM_applyFilter
Date: Thu, 26 Feb 2004 17:46:18 -0500
Reply-To: geeklog-devel at lists.geeklog.net

Dirk wrote:

> array_walk sounds useful, but I'm not sure how you would derive a common

> function from it. How would it know which parameters are supposed to be

> numeric?


I was thinking we would have to pass it an array of variables and type and
options. Maybe pass the array of variables by reference. Not sure - just
thinking out loud. Maybe this would be better as a class.

Blaine

----- Original Message ----- 
From: "Dirk Haun" <dirk at haun-online.de>
To: <geeklog-devel at lists.geeklog.net>
Sent: Thursday, February 26, 2004 5:16 PM
Subject: Re: [geeklog-devel] How to use COM_applyFilter



> Blaine,

>

> >The other purpose I had was a stronger version that only allowed "letters

> >and numbers"

> >     $p = preg_replace('/^[^(0-9a-zA-Z)]+/','', $parameter );

>

> That may make sense. We should review some of the "freedoms" that Geeklog

> is giving users for 1.3.10 anyway. E.g. user names (we've discussed this

> one before, I remember) or topic IDs.

>

>

> >if (isset($HTTP_POST_VARS['op']) ) {

> >    $op = clubApplyFilter($HTTP_POST_VARS['op']);

> >} elseif (isset($HTTP_GET_VARS['op']) ) {

> >    $op = clubApplyFilter($HTTP_GET_VARS['op']);

> >} else {

> >    $op = '';

> >}

>

> I was actually discouraging the use of this in my writings. Although it

> probably makes sense to do this for one parameter (the mode of operation,

> as in your example) and then, depending on that mode parameter, get the

> other parameters ONLY from the array they are supposed to be in. I.e.

> something like

>

>     if ($op == 'submit') {

>         // 'submit' will always indicate a POST request

>         $something = COM_applyFilter ($HTTP_POST_VARS['something']);

>     } else if ($op == 'new') {

>         $whatever = COM_applyFilter ($HTTP_GET_VARS['whatever']);

>     }

>

>

> >I have been thinking, it would be nice to be have a common function that

> >would filter all the POST and GET vars. It is possible to walk the arrays

> >and maybe send it a list of variables to clean  - using array_walk().

>

> array_walk sounds useful, but I'm not sure how you would derive a common

> function from it. How would it know which parameters are supposed to be

> numeric?

>

> bye, Dirk

>

>

> -- 

> http://www.haun-online.de/

> http://geeklog.info/

>

> _______________________________________________

> geeklog-devel mailing list

> geeklog-devel at lists.geeklog.net

> http://lists.geeklog.net/listinfo/geeklog-devel



--__--__--

Message: 5
Date: Fri, 27 Feb 2004 08:48:40 -0600
From: Tony Bibbs <tony at tonybibbs.com>
To: geeklog-users at lists.geeklog.net, Geeklog <geeklog-devel at lists.geeklog.net>
Subject: [geeklog-devel] Re: [geeklog-users] An SQL error has occured
Reply-To: geeklog-devel at lists.geeklog.net

Ah, good catch.  I read through too quick.  Anyway, the thing that 
annoys me most about Geeklog right now is the fact that topic ID's are 
text-based instead of numeric values and, unlike most other primary keys 
in Geeklog, these you can key in yourself.  This is simply bad legacy 
code from Geeklog's days of infancy.  Report this as a bug to 
http://project.geeklog.net/ and we'll need to finally make this a 
priority and get it working right.

I'm cc'ing this to the geeklog-devel list to be sure it gets seen by the 
entire crew.  Again, if you decide to look into this yourself, any help 
is appreciated.  The fix would need to occur in admin/topic.php most likely.

--Tony

Chris Besignano wrote:

> I am not using the Journal Plugin. Just staight-up geeklog. The topic I 

> was creating just happened to include the word Journal.

> 

> Tony Bibbs wrote:

> 

>> Again, note that the *fix* will happen in the journal plugin's code.  

>> If you find it and fix it please send the fix to 

>> geeklog-devtalk at lists.geeklog.net.  Thanks for looking into this...

>>

>> --Tony

>>

>> Chris Besignano wrote:

>>

>>> I realized why the error occured but was unable to resolve the issue. 

>>> Geeklog simply locked up and kept returning the SQL error no matter 

>>> which page I accessed. I agree that this is something that should be 

>>> validated. It shouldn't be much work to make it happen, maybe I'll 

>>> poke at it this weekend and add some validation code. Who do I send 

>>> my changes to?

>>>

>>> Chris Besignano

>>>

>>> Drago Goricanec wrote:

>>>

>>>> This is something geeklog should protect against. Either escape the 

>>>> data, or

>>>> validate it prior to injecting it into SQL. If there are plans to do 

>>>> this in a

>>>> future version that's fine, but I don't think it's reasonable for 

>>>> geeklog to

>>>> expect users to provide it with valid data.

>>>>

>>>> The other thing I would suggest is that either we always use POST 

>>>> methods, or

>>>> encrypt and sign the arguments generated in a GET method to avoid 

>>>> either

>>>> replaying or injecting bad data to geeklog. Nevertheless, all data 

>>>> should be

>>>> validated/sanitized prior to use.

>>>>

>>>> regards,

>>>> Drago

>>>>

>>>> Quoting Tony Bibbs <tony at tonybibbs.com>:

>>>>

>>>>  

>>>>

>>>>> the problem is the journal name has a single quote (') in it.  

>>>>> Change "Chris' Journal" to "Chris Journal" and all  would be well.

>>>>>

>>>>> --Tony

>>>>>

>>>>> Chris Besignano wrote:

>>>>>  

>>>>>

>>>>>> Hello,

>>>>>>

>>>>>> I am runnning geeklog 1.3.8-lsr4 on linux. I attempted to add a 

>>>>>> new topic, but left a space in the topic id. Now I get this SQL 

>>>>>> error and cannot access any part of the site. What can I do to 

>>>>>> recover from this? Below is a section of my error log.

>>>>>>

>>>>>>

>>>>>> Thu Feb 26 09:51:31 2004 - 1064: You have an error in your SQL 

>>>>>> syntax near 'Journal')' at line 1. SQL in question: SELECT 

>>>>>> count(*) AS count FROM gl_stories WHERE (draft_flag = 0) AND (date 

>>>>>> <= NOW()) AND (tid = 'Chris'Journal')

>>>>>> Thu Feb 26 09:51:46 2004 - 1064: You have an error in your SQL 

>>>>>> syntax near 'Journal')' at line 1. SQL in question: SELECT 

>>>>>> count(*) AS count FROM gl_stories WHERE (draft_flag = 0) AND (date 

>>>>>> <= NOW()) AND (tid = 'Chris'Journal')

>>>>>> Thu Feb 26 09:51:52 2004 - 1064: You have an error in your SQL 

>>>>>> syntax near 'Journal')' at line 1. SQL in question: SELECT 

>>>>>> count(*) AS count FROM gl_stories WHERE (draft_flag = 0) AND (date 

>>>>>> <= NOW()) AND (tid = 'Chris'Journal')

>>>>>> Thu Feb 26 09:51:56 2004 - 1064: You have an error in your SQL 

>>>>>> syntax near 'Journal')' at line 1. SQL in question: SELECT 

>>>>>> count(*) AS count FROM gl_stories WHERE (draft_flag = 0) AND (date 

>>>>>> <= NOW()) AND (tid = 'Chris'Journal')

>>>>>>

>>>>>> _______________________________________________

>>>>>> geeklog-users mailing list

>>>>>> geeklog-users at lists.geeklog.net

>>>>>> http://lists.geeklog.net/listinfo/geeklog-users

>>>>>>     

>>>>>

>>>>>

>>>>>

>>>>> _______________________________________________

>>>>> geeklog-users mailing list

>>>>> geeklog-users at lists.geeklog.net

>>>>> http://lists.geeklog.net/listinfo/geeklog-users

>>>>>

>>>>>   

>>>>

>>>>

>>>>

>>>>

>>>>

>>>> _______________________________________________

>>>> geeklog-users mailing list

>>>> geeklog-users at lists.geeklog.net

>>>> http://lists.geeklog.net/listinfo/geeklog-users

>>>>

>>>>  

>>>>

>>>

>>> _______________________________________________

>>> geeklog-users mailing list

>>> geeklog-users at lists.geeklog.net

>>> http://lists.geeklog.net/listinfo/geeklog-users

>>

>>

>> _______________________________________________

>> geeklog-users mailing list

>> geeklog-users at lists.geeklog.net

>> http://lists.geeklog.net/listinfo/geeklog-users

>>

> 

> _______________________________________________

> geeklog-users mailing list

> geeklog-users at lists.geeklog.net

> http://lists.geeklog.net/listinfo/geeklog-users



--__--__--

_______________________________________________
geeklog-devel mailing list
geeklog-devel at lists.geeklog.net
http://lists.geeklog.net/listinfo/geeklog-devel


End of geeklog-devel Digest