Description
⚠️ This issue respects the following points: ⚠️
- This is a bug, not a question or a configuration/webserver/proxy issue.
- This issue is not already reported on Github OR Nextcloud Community Forum (I've searched it).
- Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
- I agree to follow Nextcloud's Code of Conduct.
Bug description
Dispatcher should parse and validate parameter types, fail if not as declared
Expected
The dispatcher should fail calling the controller if the passed parameter type does not match the declared type.
Actual
The controller method is excuted with a wrong cast type.
Background
Coarse description of how requests are handled by controllers:
- Router finds matching route, finds controller class
- Dispatcher uses a reflector to inspect the target method arguments
- Dispatcher converts request parameters
- Dispatcher executes method on controller class
Analysis of executeController(Controller $controller, string $methodName)
The method extracts parameter values from the request and executes the looked up controller method with these parameters.
It uses a "Reflector" to determine types of the to-be-called method and tries to cast the types. It does not parse primitive values, but casts them.
Types can be specified by PHP comment annotation @type. Acceptable cast types are: int, integer, bool, boolean, float, double.
Type conversion is done via settype.
The problem
The problem with this is that wrong parameter values are silently converted to wrong values.
Passing a string notanumber to a controller method annotated with @int will result in a parameter 0 (type int) to be passed.
See also the example in the note below.
If zero is a valid value from the controller's point of view this might accidentally lead to a wrong code execution.
The proposed solution
The code shoud try to parse passed parameters and fail with an error response if parsing failed.
filter_var()alone is not suitable because it can't handle hex numbersis_numeric()alone is not suitable because it can't handle hex numbers
I'm not familiar enough with PHP to come up with a thorough solution right now, but I think this involves testing the string using Regular Expressions, converting hex to decimal using hexdec() if applicable, combined with parsing them with filter_var().
Side note on settype()
The behavior of settype() is even unpredictable as it does not properly recognize (tempted to say "parse") the type. Here's an example with PHP 8.1.2:
php > $foo = "0x20";
php > settype($foo, "int");
php > echo $foo;
0
php > $foo = "20";
php > settype($foo, "int");
php > echo $foo
20
php > $bar = 0x20;
php > echo $bar;
32
Tried Github search terms:
- is:issue controller type cast
- is:issue controller type parameter
Tried Google searches:
- nextcloud controller type parameter validate OR validation OR cast site:github.com
Steps to reproduce
- Find a controller that makes a method with
@param inttyped parameter callable via URL - Call that route and pass a string
Expected behavior
- The request fails
- The controller method is not executed
Installation method
Community Manual installation with Archive
Nextcloud Server version
29
Operating system
Debian/Ubuntu
PHP engine version
PHP 8.1
Web server
Apache (supported)
Database engine version
SQlite
Is this bug present after an update or on a fresh install?
Fresh Nextcloud Server install
Are you using the Nextcloud Server Encryption module?
None
What user-backends are you using?
- Default user-backend (database)
- LDAP/ Active Directory
- SSO - SAML
- Other
Configuration report
No response
List of activated Apps
No response
Nextcloud Signing status
No response
Nextcloud Logs
No response
Additional info
No response
Activity