[AppDB] Implement maintainer ratings.

Paul van Schayck polleke at gmail.com
Wed Jan 5 05:41:35 CST 2005


Hey Tony,

Few comments.

> +       maintainer_rating  text,
> +       maintainer_release text,

Why text, why not a varchar of 100 characters. It doesn't matter that
much but it just optimizes our code a bit.

> -        $versionName   = addslashes($_REQUEST['versionName']);
> -        $description   = addslashes($_REQUEST['description']);
> -        $webPage       = addslashes($_REQUEST['webPage']);
> +        $versionName        = addslashes($_REQUEST['versionName']);
> +        $keywords           = $_REQUEST['keywords'];
> +        $description        = addslashes($_REQUEST['description']);
> +        $webPage            = addslashes($_REQUEST['webPage']);
> +        $maintainer_rating  = $_REQUEST['maintainer_rating'];
> +        $maintainer_release = $_REQUEST['maintainer_release'];

>          //did anything change?
>          if ($VersionChanged)
>          {
>              $query = "UPDATE appVersion SET versionName = '".$versionName."', ".
>                  "keywords = '".$_REQUEST['keywords']."', ".
>                  "description = '".$description."', ".
> -                "webPage = '".$webPage."'".
> +                "webPage = '".$webPage."',".
> +                "maintainer_rating = '".$maintainer_rating."',".
> +                "maintainer_release = '".$maintainer_release."'".
>                  " WHERE appId = ".$_REQUEST['appId']." and versionId = ".$_REQUEST['versionId'];
>              if (mysql_query($query))
>              {

This is far from sql injection safe. Anyone can enter anything into
$_REQUEST['mantainer_release'] or $_REQUEST['mantainer_rating'] and
change any field from the appVersion table. It isn't a real problem in
this case because they can already change most fields as maintainer.
But they can for example change the versionId to make an app-version
belong to any application.

To prevent this we should make our queries with the compile_*_string
functions from include/db.php. That function is doing addslashes. And
generates an as safe as possible query string. Usage can be seen in
admin/editAppFamily.php.

Also query_appdb should be used instead of mysql_query().

And appId and versionId should always be check with is_numeric().
Right now without giving out details it's possible to change any
application as long as you are maintainer of one application.

Will you fix it or shall I?

Paul



More information about the wine-devel mailing list