Taint can't be transmitted through numerics nor bool

[orklah]

This will fix #6991 and fix #6992 (sorry, I'm not sure how to contribute to a previously made PR, so I just stole the tests :D)

We could improve this PR in the future: It's the second time I list int, bool and float for taint purpose. We could create a method on Union that would take a type and return if it can be a vector for taints (for example, we can exclude bool, int, float, probably numeric and numeric-string. Other are more unclear: what about resource? callable-string? etc...). It would also allow checking union types...

Once again, I was surprised, once I fixed the parameter type, the cast solved itself and it did not allow taint to pass anymore. I'm not completely sure what's going on here.

[ohader]

Thx, I also continued a bit on this topic (still having limited knowledge about many details in Psalm internals...):

• ohader@d43b902 - test case for bool type hint in function signature
• ohader@1db0f05 - "bug fix" (maybe)
  • added isBool() check
  • basically using $function_param->signature_type instead of $function_param->type like in @orklah's change

[ohader]

It's the second time I list int, bool and float for taint purpose

Probably we might use some trait here that collects these kind of common checks - finding good names for all this functions is the actual and real challenge then... 😬

[orklah]

No need for traits, we have a Union class that can carry this kind of methods

isVectorForTaints would be good I think :)

The hard part will be listing every type and thinking about whether it can transmit any type of taint or not !

[weirdan]

I'm not sure how to contribute to a previously made PR

It's easy with gh cli tool: you just check out the PR with gh pr checkout <pr number> and then edit, commit and push as usual.

There's also 'Code' button in the top right corner of PR web interface, that, once clicked, show alternative methods:

[orklah]

Thx, I also continued a bit on this topic

Sorry, I didn't know you were going to work on it, I'd have let you do it instead :)

[orklah]

it's easy with gh cli tool

Thanks! Will try it next time!

[orklah]

I'm merging that, feel free to improve onto it if needed :)

[ohader]

Thx, I also continued a bit on this topic

Sorry, I didn't know you were going to work on it, I'd have let you do it instead :)

No problem at all & I should have mentioned that I'd continue. Anyways, we both were digging into the topic to get it resolved. That's the important part & thanks for that! 👍

The hard part will be listing every type and thinking about whether it can transmit any type of taint or not !

I was thinking about adding $type->isEmpty() and $type->isNull() as well... but that really should be covered with more tests... I could work on extending the behavior using a separate PR/issue.

[orklah]

@ohader
about what you said here:

basically using $function_param->signature_type instead of $function_param->type like in @orklah's change

This was on purpose and I described it in the previous PR:

Note also that this PR represent a subtle change in behaviour: declaring that a function takes an int as a param or gives int as a return type in phpdoc will now remove possible taints. It may be possible to do so only if the type does not come from docblock but it seemed more aligned with Psalm's policy to trust types even in docblock

It's a slight risk, but not much considering all the checks Psalm does

[orklah]

I was thinking about adding $type->isEmpty() and $type->isNull() as well

Ideally, we should consider every type inside an Union. You can take isAlwaysTrue as an example. Basically bool|null should also be excluded from taint analysis