Skip to content
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

Taint mode in PHP doesn't work on object that method is called on #4320

Closed
Sjord opened this issue Nov 22, 2021 · 0 comments · Fixed by #4323
Closed

Taint mode in PHP doesn't work on object that method is called on #4320

Sjord opened this issue Nov 22, 2021 · 0 comments · Fixed by #4323
Assignees
Labels
bug Something isn't working feature:taint lang:php

Comments

@Sjord
Copy link
Contributor

Sjord commented Nov 22, 2021

https://semgrep.dev/s/sjord:domdocument_load

  pattern-sources:
  - patterns:
    - pattern: new DOMDocument(...)
  pattern-sinks:
  - patterns:
    - pattern-inside: $DOMDOCUMENT->load($FILENAME, ...)
    - pattern: $DOMDOCUMENT
<?php
$doc = new DOMDocument();
$doc->load('file.xml');  // I expect this to match, it doesn't
$doc->load($doc);  // I expect this to match, it does
$something->load($doc);  // This doesn't match, as expected.

I try to match calls to DOMDocument::load, with taint mode to track whether $DOMDOCUMENT is the result of a call to new DOMDocument(), in $DOMDOCUMENT->load($FILENAME, ...). However, this doesn't seem to work properly. $doc->load('file.xml') doesn't match, even though $doc should be tainted here.

@brendongo brendongo added bug Something isn't working feature:taint lang:php labels Nov 22, 2021
@IagoAbal IagoAbal self-assigned this Nov 23, 2021
IagoAbal added a commit that referenced this issue Nov 23, 2021
If the LHS of `->foo` was a sink, and we encountered `x->foo` where `x`
was a variable, we did not report a finding even if `x` was tainted. We
were not checking whether `x` was in a sink position.

Although they are not represented as such in the IL, variables are
themselves subexpressions, so we must check whether they are sinks or
sanitized.

Closes #4320

test plan:
make test # test included
IagoAbal added a commit that referenced this issue Nov 30, 2021
If the LHS of `->foo` was a sink, and we encountered `x->foo` where `x`
was a variable, we did not report a finding even if `x` was tainted. We
were not checking whether `x` was in a sink position.

Although they are not represented as such in the IL, variables are
themselves subexpressions, so we must check whether they are sinks or
sanitized.

Closes #4320

test plan:
make test # test included
IagoAbal added a commit that referenced this issue Nov 30, 2021
If the LHS of `->foo` was a sink, and we encountered `x->foo` where `x`
was a variable, we did not report a finding even if `x` was tainted. We
were not checking whether `x` was in a sink position.

Although they are not represented as such in the IL, variables are
themselves subexpressions, so we must check whether they are sinks or
sanitized.

Closes #4320

test plan:
make test # test included
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature:taint lang:php
Development

Successfully merging a pull request may close this issue.

3 participants