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