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 numbers
is_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:
⚠️ This issue respects the following points: ⚠️
Bug description
Dispatcher
should parse and validate parameter types, fail if not as declaredExpected
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:
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 parameter0
(typeint
) 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 numbersI'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 withfilter_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:Tried Github search terms:
Tried Google searches:
Steps to reproduce
@param int
typed parameter callable via URLExpected behavior
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?
Configuration report
No response
List of activated Apps
No response
Nextcloud Signing status
No response
Nextcloud Logs
No response
Additional info
No response