-
Notifications
You must be signed in to change notification settings - Fork 17
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
Comments
@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
I guess the unique_id contains chars and integers so we can't validate here ;)
This is a valid php Version (somewhere in the future)
This is a supported database
This is a valid Version number for a database maybe we should just allow our minimums here.
This is a valid Joomla Version number (somewhere in the future) ;)
We can't validate here. Feel free to add code to the validation ;) |
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. |
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. |
@dneukirchen i have just send a fix for Joomla version (#25) can you propose a fix for the php Version? |
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. |
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. |
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. |
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). |
#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. |
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. |
Steps to reproduce the issue
Send a post request with invalid / unrealistic data.
Fields that are not validated correctly:
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 ...
The text was updated successfully, but these errors were encountered: