tysonandre / psalm Goto Github PK

E.g. dump a list of nodes that contain sources/sinks, and the edges that are connected to sources and/or sinks directly or indirectly, as well as what are escaped/removed/added by each function call.

This would help in checking that psalm is actually aware of the things that developers would consider sources/sinks that could be automatically inferred in a project.

Not sure if broadly useful

Not sure how best to handle this. I only skimmed the code, so my understanding might be limited. This doesn't contain a solution yet.

Possible ideas:

Only emit FalseableReturnStatement if at least one of the types overlaps with the return type, and false is another possible type that doesn't overlap.
Only emit FalseableReturnStatement if false is the only type which isn't allowed in the desired return type?

To do this, maybe TypeChecker could partition types into types which are subsumed, types which are completely invalid, and types which are less specific?

A bug was filed upstream.

Maybe make issueBuffer::accept always return false, so that I don't have to remove all of the early returns.

E.g. great for performance, but causes unpredictable crashes.

Continue using composer's autoloader, and support autoloader which returns a file, but doesn't actually require_once/require it

60MB (igbinary) vs 240MB (default).

Also, look into tuning CFLAGS used by pecl install, they seem to be unoptimized

This will make this fork less likely to break in the future.

The third party composer dependencies look like they may make that unviable (and break after composer upgrades), unless releases get limited to Phars or include composer.lock

This tool may potentially save some time in maintaining backports (and allow you to adopt php 7 syntax in master faster).

I worked on https://github.com/TysonAndre/Transphpile while looking at generating backports for a large php project. I had some fixes for the upstream that weren't yet merged.

That project can generate polyfills for param checks, return type checks, the ?? operator, (new Visitor())(), etc.

• This would allow using php 7 features

Limitations:

• Classes that don't exist in 5.6 require manual patching or polyfills
• Some bugs in uncommon php syntax

convert_php70_to_php56.sh is an example of a script used to generate patches.

Example of how this would be used:

1. Create a php56-uncompiled branch. Add any manual workarounds or stubs there, along with the script to convert to php 5.6.

   PHP-Parser 4.x depends on php 7.1. It might be possible to run Transphpile on nikic/php-parser (either at runtime or as part of the composer.phar install)

2. Generate release branches by merging master into php56-uncompiled, creating a new branch, verifying that unit tests pass

It gets in the way of setting up a new project.

Convert these to suppressable issues instead.

See #1

Avoids potential for problems with #9 and cleanup tasks

So far, this isn't important, and I've worked around it by dockerizing and using a different root volume.

Avoid accidentally sharing cache between different versions of psalm.

(When the union types overlap, but some types are invalid (e.g. passing string|false when string is expected))

name tbd

https://github.com/TysonAndre/psalm/pulls

Of limited general use. It's used in one project.

https://secure.php.net/manual/en/runkit.configuration.php

<?php // --taint-analysis

// Has dynamic properties
class Other {
}

class X {
  /** @var Other */
  public $var;

  public function __construct(Other $x) {
    $this->var = $x;
  }

  public function set($name, $value) {
    $this->var->$name = $value;
  }
}

$x = new X();
$x->set('myname', $_GET['name']);
echo $x->var->myname;

calling X->set() should affect the taintedness of uses of OtherObject.

Hook\AfterMethodCallAnalysisInterface seems like the best way to do this.

See upstream

It's possible for psalm to infer that individual array parameters are tainted based on calls.

Marking array['bad']-src/taint_mixed.php:195-216 as a source with the desired taint types (e.g. html for raw html) may be useful for API declarations that take dynamic input with configurable filters (along the lines of https://github.com/TysonAndre/psalm/blob/master/examples/plugins/APIFilterPlugin.php )

<?php
/**
 * @param array<string,string> $value
 */
function test_taint(array $value, array $other = []) {
    eval(htmlentities($value['bad']));
}
test_taint([
    'good' => 'some literal',
    'bad' => $_GET['evil'],
]);

With manual logging of the node ids:

../psalm/psalm --taint-analysis
Scanning files...
Analyzing files...

░Sources:
$_GET:src/taint_mixed.php:204
Edges:
From test_taint#1
-> $value['bad']-src/taint_mixed.php:129-134
From call to htmlentities-src/taint_mixed.php:129-141
-> htmlentities#1-src/taint_mixed.php:116
From $value['bad']-src/taint_mixed.php:129-134
-> call to htmlentities-src/taint_mixed.php:129-141
From htmlentities#1-src/taint_mixed.php:116
-> htmlentities-src/taint_mixed.php:116(escape html)
From htmlentities-src/taint_mixed.php:116
-> eval#1-src/taint_mixed.php:116
From $_GET:src/taint_mixed.php:204
-> $_GET['evil']-src/taint_mixed.php:204-208
From $_GET['evil']-src/taint_mixed.php:204-208
-> array['bad']-src/taint_mixed.php:195-216
From call to test_taint-src/taint_mixed.php:159-219
-> test_taint#1
From array['bad']-src/taint_mixed.php:195-216
-> call to test_taint-src/taint_mixed.php:159-219
Sinks:
eval#1-src/taint_mixed.php:116


ERROR: TaintedInput - src/taint_mixed.php:6:10 - Detected tainted text in path: $_GET -> $_GET['evil'] (src/taint_mixed.php:10:14) -> array['bad'] (src/taint_mixed.php:10:5) -> call to test_taint (src/taint_mixed.php:8:12) -> test_taint#1 (src/taint_mixed.php:5:27) -> $value['bad'] (src/taint_mixed.php:6:23) -> call to htmlentities (src/taint_mixed.php:6:23) -> htmlentities#1 (src/taint_mixed.php:6:23) -> htmlentities (src/taint_mixed.php:6:10) -> eval#1 (src/taint_mixed.php:6:10) (see https://psalm.dev/205)
    eval(htmlentities($value['bad']));

Type\Union->from_phpdoc should give enough information to know if the cast was unnecessary.

Check if this makes sense to do