From 302723947f9ac579f8597902ec1924867b1f0620 Mon Sep 17 00:00:00 2001 From: RobertWesner Date: Mon, 19 May 2025 17:16:48 +0200 Subject: [PATCH 1/2] Fixed bypassing novarity check via ${'var_name'} --- .gitignore | 1 + .idea/inspectionProfiles/Project_Default.xml | 6 ++++ src/Autoload/Loader.php | 2 +- src/Internal/Import.php | 28 ++++++++++++++++--- tests/Fixture/evil_bypass.php | 3 ++ tests/Fixture/evil_class.php | 20 +++++++++++++ tests/Fixture/evil_static_class.php | 12 ++++++++ .../parameters_are_a_necessary_evil.php | 23 +++++++++++++++ tests/Test/Internal/ImportTest.php | 20 +++++++++++++ 9 files changed, 110 insertions(+), 5 deletions(-) create mode 100644 .idea/inspectionProfiles/Project_Default.xml create mode 100644 tests/Fixture/evil_bypass.php create mode 100644 tests/Fixture/evil_class.php create mode 100644 tests/Fixture/evil_static_class.php create mode 100644 tests/Fixture/parameters_are_a_necessary_evil.php diff --git a/.gitignore b/.gitignore index fc2162e..8492f2d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /vendor/ composer.lock .phpcs.cache +.phpunit.cache diff --git a/.idea/inspectionProfiles/Project_Default.xml b/.idea/inspectionProfiles/Project_Default.xml new file mode 100644 index 0000000..b49ef85 --- /dev/null +++ b/.idea/inspectionProfiles/Project_Default.xml @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/src/Autoload/Loader.php b/src/Autoload/Loader.php index 16f7e1b..173381f 100644 --- a/src/Autoload/Loader.php +++ b/src/Autoload/Loader.php @@ -25,7 +25,7 @@ public static function load(): void array_map(fn () => require_once func_get_arg(0), self::DEPENDENCIES); // now that the Import is available, we can retroactively proof novarity - array_map(Import::enforceNovarity(...), self::DEPENDENCIES); +// array_map(Import::enforceNovarity(...), self::DEPENDENCIES); } })::load(); diff --git a/src/Internal/Import.php b/src/Internal/Import.php index 3f8b01a..5362763 100644 --- a/src/Internal/Import.php +++ b/src/Internal/Import.php @@ -20,13 +20,32 @@ final class Import public static function enforceNovarity(): false { // TODO: block eval if it contains any of the tokens - // TODO: does this already block object properties? more tests -> more better // Use Exception directly without Novara:: prefix as to enable autoload return Exception::throwIf( count( array_filter( - PhpToken::tokenize(file_get_contents(func_get_arg(0))), - fn () => func_get_arg(0)->is([ + explode( + ',', + str_replace( + // ${ + '36,123,', + 'BAD,', + preg_replace( + // Allow function parameters without using them to enable + // interfaces and their implementation. + '/' . T_FUNCTION . ',' . T_WHITESPACE . ',(\d+,)+' . '123' . '/', + 'ALLOWED', + implode( + ',', + array_map( + fn () => func_get_arg(0)->id, + PhpToken::tokenize(file_get_contents(func_get_arg(0))), + ), + ), + ), + ), + ), + fn () => in_array(func_get_arg(0), [ T_VARIABLE, T_GLOBAL, T_ENCAPSED_AND_WHITESPACE, @@ -34,8 +53,9 @@ public static function enforceNovarity(): false T_DOLLAR_OPEN_CURLY_BRACES, T_NUM_STRING, T_STRING_VARNAME, + 'BAD', ]), - ), + ) ) > 0, new NovarityNotMetException(sprintf( 'File "%s" contains variables. Unforgivable!', diff --git a/tests/Fixture/evil_bypass.php b/tests/Fixture/evil_bypass.php new file mode 100644 index 0000000..0522fb8 --- /dev/null +++ b/tests/Fixture/evil_bypass.php @@ -0,0 +1,3 @@ +out = func_get_arg(0); + + return $this->out; + } +} diff --git a/tests/Fixture/evil_static_class.php b/tests/Fixture/evil_static_class.php new file mode 100644 index 0000000..54bcd36 --- /dev/null +++ b/tests/Fixture/evil_static_class.php @@ -0,0 +1,12 @@ + [ + false, + 'require', + __DIR__ . '/../../Fixture/parameters_are_a_necessary_evil.php' + ], + 'evil class' => [ + true, + 'require', + __DIR__ . '/../../Fixture/evil_class.php', + ], + 'evil static class' => [ + true, + 'require', + __DIR__ . '/../../Fixture/evil_static_class.php', + ], + 'evil bypass' => [ + true, + 'require', + __DIR__ . '/../../Fixture/evil_bypass.php', + ], ]; } From 2bcc3142f87f8d4c8ea681bab289fd06b095dd3c Mon Sep 17 00:00:00 2001 From: RobertWesner Date: Mon, 19 May 2025 17:20:27 +0200 Subject: [PATCH 2/2] Fixed workflows --- .github/workflows/coverage.yml | 2 -- .github/workflows/tests.yml | 4 +--- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 57aa304..5c429fb 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -1,7 +1,5 @@ name: tests -# TODO: https://docs.github.com/en/actions/sharing-automations/reusing-workflows - on: push: branches: [ 'main' ] diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c2dd68e..ce156d5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,8 +1,6 @@ name: tests -on: - pull_request: - types: [opened, reopened] +on: pull_request permissions: contents: read