Skip to content

Conversation

LukasRos
Copy link

I've extended the code to check for whitelisted types and interfaces in parameter type hints and use statements. Even though this is not strictly required for practical sandbox constraints; for my use of this library this is very helpful. I'm not sure if this breaks other use cases; if so I can add a switch.

$node->type = new Node\Name($this->sandbox->getDefinedClass($class));
}
if ($this->sandbox->isWhitelistedInterface($class))
$this->sandbox->checkInterface($class);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CS :s

} else {
$this->sandbox->validationError("Sandboxed code attempted use invalid namespace or alias!", Error::DEFINE_ALIAS_ERROR, $node);
}
if ($this->sandbox->isWhitelistedInterface($use->alias))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CS :s

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what does "CS :s" mean?
Also, I think I still have some flawed logic in this, I will work on this again and update the PR later.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Coding style": if ($this->sandbox->isWhitelistedInterface($use->alias)) { ;)

* Test whether sandbox disallows non-whitelisted classes in parameter type hints
*/
public function testDisallowsTypeInParam(){
$this->expectException('PHPSandbox\Error');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sur, but use statement and Error::class ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test I'm expecting the use statement to fail even though use statements are allowed because the class I'm using is not whitelisted.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean:
in top of file:
use PHPSandbox\Error;

And line 448:
$this->expectException(Error::class);

@peter279k
Copy link
Contributor

The tests/DefaultConfigTest.php file has been conflicts and it should be concerned.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@squash-labs
Copy link

squash-labs bot commented Jan 24, 2023

Manage this branch in Squash

Test this branch here: https://master-63c39.squash.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants