Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Input validation #24

Open
dneukirchen opened this issue Mar 30, 2016 · 10 comments
Open

Input validation #24

dneukirchen opened this issue Mar 30, 2016 · 10 comments

Comments

@dneukirchen
Copy link

Steps to reproduce the issue

Send a post request with invalid / unrealistic data.

curl -X POST -F "unique_id=example5unique7id2425" -F "php_version=7.9" -F "db_type=mysql" -F "db_version=1.0" -F "cms_version=3.9" -F "server_os=something" "http://developer.joomla.org/stats/submit"

Fields that are not validated correctly:

  • php_version (all between 5.3.1 and 8.0.0 (except major 6) is allowed).
  • cms_version
  • db_version
  • server_os

Expected result

Some kind of validation/general error. whitelist/blacklist validation.

Actual result

Data is not validated correctly and will be saved.

Additional comments

sorry for some test posts ...

@zero-24
Copy link

zero-24 commented Mar 30, 2016

@dneukirchen can you have a look here: https://github.com/joomla-extensions/jstats-server/blob/master/src/Controllers/SubmitControllerCreate.php#L62-L124

We have some data validation. The only point we can't validate is server_os and unique_id

"unique_id=example5unique7id2425"

I guess the unique_id contains chars and integers so we can't validate here ;)

"php_version=7.9"

This is a valid php Version (somewhere in the future)

"db_type=mysql"

This is a supported database

"db_version=1.0"

This is a valid Version number for a database maybe we should just allow our minimums here.

"cms_version=3.9"

This is a valid Joomla Version number (somewhere in the future) ;)

"server_os=something"

We can't validate here.

Feel free to add code to the validation ;)

@mbabker
Copy link

mbabker commented Mar 30, 2016

Short of introducing strict validation rules that requires someone updating them every time a new Joomla, MySQL, MariaDB (because they use a different version numbering schema than MySQL), SQL Server, PostgreSQL, or PHP release is issued, I honestly don't have any suggestions on this. And even I don't pay enough attention to the release schedules of all of these platforms to do this kind of strict management of versions.

@dneukirchen
Copy link
Author

try cms_version 3.1.2.3.4.5.6.7 or mysql 1.0 (unrealistic, not supported) or just look at https://developer.joomla.org/stats/ (edit: cleaned up know ;-))

I agree that for some of the fields the validation is not easy or too expensive. But we could at least validate the cms and php version better.

Or find another way to prevent manipulation and spamming.

@zero-24
Copy link

zero-24 commented Mar 30, 2016

@dneukirchen i have just send a fix for Joomla version (#25) can you propose a fix for the php Version?

@dneukirchen
Copy link
Author

I will look into this at the j!dev weekend in germany next week. For know the only thing i can imagine is some kind of white/blacklist. But as michael said, this is horrible in terms of maintenance.

@mbabker
Copy link

mbabker commented Mar 30, 2016

For PHP 5.3 and 5.4 we possibly could do some sort of validation on versions as those branches are EOL, 5.5 is about to be too. 5.6 and 7.x being active development/support makes that a headache. I guess the same can be said for MySQL 5.0, maybe 5.1 too (their release notes don't indicate support ended though like 5.0's did).

Validation rules should also take into account each product's master branch. Though unreleased PHP 7.1 has 9 entries in the stats database and that is a valid entry as it's based on a development head.

@dneukirchen
Copy link
Author

Black/Whitelists for the php, cms and db versions are not maintainable, if we dont pull them automatically. Any (other) ideas?

For the operating system we could do a whitelist of the most used systems and some kind of notification logic for entries that are not in the list.

@mbabker
Copy link

mbabker commented May 7, 2016

The kind folks on Twitter reminded me that we can make use of GitHub's API and pull data from the PHP source mirror and that'll give us a list of valid PHP releases. We'd just need to add to our allowed version list the latest patch release + 1 (representing dev branch, so 5.4.46 would be OK) and the last minor release + 1 (so 7.1.0 is OK since that's PHP's master right now) and I think that would cover all cases for that software stream.

Could do the same for Joomla version numbers (parse from git tags).

@mbabker
Copy link

mbabker commented May 7, 2016

#28 runs with the idea for processing the git repo tags and starts with the CMS' own data. We can fine tune this then work on the implementation for PHP versions.

@mbabker
Copy link

mbabker commented Sep 22, 2016

We have PHP and Joomla version validation in place now. The only other sane thing I can think of, which still requires some manual maintenance I guess (compared to how the first two were done with only needing a simple CLI command run) is a method validating the database type and version and applying some known rules against that (i.e. if MySQL it has to be 5.0.4+, probably not a 5.2-4 number since I don't believe MySQL ever used those for anything publicly released, 5.7 the last allowed 5.x number, etc.). Otherwise, this is as good as complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants