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
