Check the direct-access guard by tokenising, not by grepping for it
The check asked whether "defined( 'ABSPATH' )" appeared anywhere in the file. A comment containing those words therefore passed it. Measured with a planted file whose only mention was in a comment: 15 shipped PHP files examined, every one guarded Green over a file with no guard at all — one that runs for anyone who fetches it directly, outside WordPress and outside every capability check the code assumes has already happened. This is the check with the worst consequence of being hollow, and it was hollow in the one way a grep always can be. From the cache session by way of security: a check that must RECOGNISE its own code has one blind spot per recognition method, and one that reads what the source actually says has none of that kind. PHP's tokeniser cannot be fooled by a comment, because it is the thing that decides what a comment is. ci/check-abspath.php parses each shipped file and looks for a real defined() call on ABSPATH or WP_UNINSTALL_PLUGIN. Comments and doc blocks are tokens of their own and are simply never examined. The count and the floor stay, for the reason every check here has them. Verified through the container path with php hidden from `command -v`, since static.sh runs on a Gitea runner that has no php: identical output either way. A 31st self-test assertion plants the same fixture. It is grade 2 evidence — a planted instance of exactly the failure the check describes, rather than an arbitrary trigger that only proves the check can go red at all. Two things that assertion caught about itself on its first run: The check's success line now says "parsed" where it said "examined", and the existing assertion on the count was still matching the old word. It went red rather than quietly matching nothing, which is why the wording is asserted at all. And `static.sh | grep -q` reports STATIC.SH's exit status under `set -o pipefail`, not grep's — and static.sh is expected to fail in that fixture, so the assertion read "not found" however the check behaved. The mirror image of the `go test | grep` trap already in the playbook. Output is captured to a variable first now, with the reason inline. Swept the kit for text belonging to other plugins while here, after cache found a comment in its copy describing security's tree: no NLSEC_/NLCAC_ references, and every path named in ci/ and tests/ either exists here or is WordPress core. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
e20c8522b7
commit
d1703dcc03
@@ -0,0 +1,98 @@
|
|||||||
|
<?php
|
||||||
|
/**
|
||||||
|
* Verify that every shipped PHP file really guards against direct access.
|
||||||
|
*
|
||||||
|
* 🔴 Tokenised, not grepped, and that is the whole point of the file.
|
||||||
|
*
|
||||||
|
* The grep version asked whether "defined( 'ABSPATH' )" appeared ANYWHERE in
|
||||||
|
* the file. A comment saying the guard is handled elsewhere therefore passed,
|
||||||
|
* and so did a file with the words and no guard at all. Measured: a planted
|
||||||
|
* file whose only mention was in a comment produced
|
||||||
|
* "15 shipped PHP files examined, every one guarded" — green over a file that
|
||||||
|
* runs for anyone who fetches it directly, outside WordPress and outside every
|
||||||
|
* capability check the code assumes has already happened.
|
||||||
|
*
|
||||||
|
* A check that must RECOGNISE its own code has one blind spot per recognition
|
||||||
|
* method; one that reads what the source actually says has none of that
|
||||||
|
* particular kind. PHP's own tokeniser cannot be fooled by a comment, because
|
||||||
|
* it is the thing that decides what a comment is.
|
||||||
|
*/
|
||||||
|
|
||||||
|
$min = 0;
|
||||||
|
$files = array();
|
||||||
|
foreach ( array_slice( $argv, 1 ) as $arg ) {
|
||||||
|
if ( 0 === strpos( $arg, '--min=' ) ) {
|
||||||
|
$min = (int) substr( $arg, 6 );
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
$files[] = $arg;
|
||||||
|
}
|
||||||
|
|
||||||
|
$guards = array( 'ABSPATH', 'WP_UNINSTALL_PLUGIN' );
|
||||||
|
$problems = 0;
|
||||||
|
$checked = 0;
|
||||||
|
|
||||||
|
foreach ( $files as $path ) {
|
||||||
|
if ( ! is_readable( $path ) ) {
|
||||||
|
printf( "::error file=%s::Listed as shipped but not readable.\n", $path );
|
||||||
|
++$problems;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
++$checked;
|
||||||
|
$tokens = token_get_all( file_get_contents( $path ) );
|
||||||
|
$count = count( $tokens );
|
||||||
|
$found = false;
|
||||||
|
|
||||||
|
for ( $i = 0; $i < $count; $i++ ) {
|
||||||
|
$token = $tokens[ $i ];
|
||||||
|
|
||||||
|
// Comments and doc blocks are tokens of their own, so they are simply
|
||||||
|
// never examined. That is the property the grep version lacked.
|
||||||
|
if ( ! is_array( $token ) || T_STRING !== $token[0] || 'defined' !== strtolower( $token[1] ) ) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// defined ( 'ABSPATH' ) — whitespace anywhere, quotes either kind.
|
||||||
|
$j = $i + 1;
|
||||||
|
while ( $j < $count && is_array( $tokens[ $j ] ) && T_WHITESPACE === $tokens[ $j ][0] ) {
|
||||||
|
++$j;
|
||||||
|
}
|
||||||
|
if ( $j >= $count || '(' !== $tokens[ $j ] ) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
++$j;
|
||||||
|
while ( $j < $count && is_array( $tokens[ $j ] ) && T_WHITESPACE === $tokens[ $j ][0] ) {
|
||||||
|
++$j;
|
||||||
|
}
|
||||||
|
if ( $j < $count && is_array( $tokens[ $j ] ) && T_CONSTANT_ENCAPSED_STRING === $tokens[ $j ][0] ) {
|
||||||
|
if ( in_array( trim( $tokens[ $j ][1], "'\"" ), $guards, true ) ) {
|
||||||
|
$found = true;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if ( ! $found ) {
|
||||||
|
printf(
|
||||||
|
"::error file=%s,line=1::No guard against direct access. Add: if ( ! defined( 'ABSPATH' ) ) { exit; }\n",
|
||||||
|
$path
|
||||||
|
);
|
||||||
|
++$problems;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The floor, for the same reason every other check in this kit has one: "every
|
||||||
|
// shipped file is guarded" is also what zero shipped files prints.
|
||||||
|
if ( $checked < $min ) {
|
||||||
|
fwrite( STDERR, sprintf( "Only %d shipped PHP file(s) were examined, expected at least %d. The file list is wrong, so this check proved nothing.\n", $checked, $min ) );
|
||||||
|
exit( 1 );
|
||||||
|
}
|
||||||
|
|
||||||
|
if ( $problems > 0 ) {
|
||||||
|
printf( "\n%d of %d shipped PHP files have no guard against direct access.\n", $problems, $checked );
|
||||||
|
exit( 1 );
|
||||||
|
}
|
||||||
|
|
||||||
|
printf( " %d shipped PHP files parsed, every one guards against direct access\n", $checked );
|
||||||
|
exit( 0 );
|
||||||
+30
-1
@@ -127,10 +127,39 @@ for f in control-hash.txt control-placeholder.txt control-masked.txt \
|
|||||||
done
|
done
|
||||||
|
|
||||||
# The guard check must report a plausible number rather than a bare adjective.
|
# The guard check must report a plausible number rather than a bare adjective.
|
||||||
echo "$out" | grep -qE '^ [0-9]+ shipped PHP files examined' \
|
echo "$out" | grep -qE '^ [0-9]+ shipped PHP files parsed' \
|
||||||
&& ok "the direct-access check reports a count" \
|
&& ok "the direct-access check reports a count" \
|
||||||
|| bad "the direct-access check printed no count — it cannot tell 13 files from 0"
|
|| bad "the direct-access check printed no count — it cannot tell 13 files from 0"
|
||||||
|
|
||||||
|
# --------------------------------------------------- direct-access guard
|
||||||
|
# 🔴 Grade 2 evidence: a planted instance of exactly the failure the check
|
||||||
|
# describes, not an arbitrary trigger. The grep version of this check passed
|
||||||
|
# this fixture — the words are present, in a comment, and there is no guard.
|
||||||
|
# It reported "every one guarded" over a file that runs for anyone who fetches
|
||||||
|
# it directly.
|
||||||
|
echo
|
||||||
|
echo "→ the direct-access check reads structure, not text"
|
||||||
|
cat > includes/class-selftest-unguarded.php <<'FIXTURE'
|
||||||
|
<?php
|
||||||
|
// This file is safe because defined( 'ABSPATH' ) is checked elsewhere.
|
||||||
|
// It is not. There is no guard anywhere below this line.
|
||||||
|
echo 'reachable';
|
||||||
|
FIXTURE
|
||||||
|
git add -N includes/class-selftest-unguarded.php >/dev/null
|
||||||
|
# Captured first, NOT piped into grep. This script runs under `set -o
|
||||||
|
# pipefail`, and static.sh is expected to exit non-zero here — so
|
||||||
|
# `static.sh | grep -q` would report STATIC.SH's status, not grep's, and the
|
||||||
|
# assertion would read "not found" however the check behaved. The mirror image
|
||||||
|
# of the `go test | grep` trap, and it cost this assertion one red run.
|
||||||
|
guard_out=$(./ci/static.sh 2>&1 || true)
|
||||||
|
if printf '%s' "$guard_out" | grep -q 'class-selftest-unguarded.php,line=1::No guard'; then
|
||||||
|
ok "a file mentioning the guard only in a comment is still flagged"
|
||||||
|
else
|
||||||
|
bad "an unguarded file passed because the words appear in a comment"
|
||||||
|
fi
|
||||||
|
rm -f includes/class-selftest-unguarded.php
|
||||||
|
git rm -q --cached includes/class-selftest-unguarded.php >/dev/null 2>&1 || true
|
||||||
|
|
||||||
# ---------------------------------------------------------------- text domain
|
# ---------------------------------------------------------------- text domain
|
||||||
# 🔴 ci_php, never a bare `php`. The Gitea runner image has NO php — that is
|
# 🔴 ci_php, never a bare `php`. The Gitea runner image has NO php — that is
|
||||||
# why ci_php exists and says so — and this section originally called `php`
|
# why ci_php exists and says so — and this section originally called `php`
|
||||||
|
|||||||
+17
-18
@@ -50,27 +50,26 @@ done
|
|||||||
rm -f "$tarball"
|
rm -f "$tarball"
|
||||||
|
|
||||||
ci_step "protection against direct access"
|
ci_step "protection against direct access"
|
||||||
before=$fails
|
|
||||||
# Without the guard a PHP file can be requested straight from the browser,
|
# Without the guard a PHP file can be requested straight from the browser,
|
||||||
# outside WordPress and therefore outside every capability check the code
|
# outside WordPress and therefore outside every capability check the code
|
||||||
# assumes has already happened.
|
# assumes has already happened.
|
||||||
guard_scanned=0
|
#
|
||||||
while read -r f; do
|
# 🔴 Tokenised by ci/check-abspath.php, not grepped. The grep version asked
|
||||||
[ -n "$f" ] || continue
|
# whether the words appeared anywhere in the file, so a COMMENT saying the
|
||||||
guard_scanned=$((guard_scanned + 1))
|
# guard was handled elsewhere passed it. Measured with a planted file whose
|
||||||
grep -q "defined( 'ABSPATH' )\|defined( 'WP_UNINSTALL_PLUGIN' )" "$f" \
|
# only mention was in a comment: "15 shipped PHP files examined, every one
|
||||||
|| { ci_annotate error "$f" 1 "Missing the guard against direct access: if ( ! defined( 'ABSPATH' ) ) { exit; }"; fails=$((fails + 1)); }
|
# guarded" — green over a file with no guard at all.
|
||||||
done <<< "$(shipped_files)"
|
#
|
||||||
# 🔴 The count is the result, not decoration. "every shipped file is guarded"
|
# The count and the floor stay: "every shipped file is guarded" is also what
|
||||||
# is also what ZERO shipped files prints, and shipped_files() swallows a
|
# zero shipped files prints, and shipped_files() swallows a dist-files.sh
|
||||||
# dist-files.sh failure with `|| true`, so an empty list is one broken
|
# failure with `|| true`. The floor is above 1 because a bad .distignore
|
||||||
# .distignore away. The floor is above 1 on purpose: a pattern that wrongly
|
# pattern can collapse the list without emptying it.
|
||||||
# excludes includes/ leaves the main file behind and collapses the list
|
shipped_php=$(shipped_files)
|
||||||
# without emptying it. Proven by stubbing ci/dist-files.sh to print nothing.
|
if [ -z "$shipped_php" ]; then
|
||||||
if [ "$guard_scanned" -lt "$MIN_SHIPPED_PHP_FILES" ]; then
|
note_fail "No shipped PHP files were listed at all. The file list is wrong, so this check proved nothing."
|
||||||
note_fail "Only $guard_scanned shipped PHP file(s) were examined for the direct-access guard, expected at least $MIN_SHIPPED_PHP_FILES. The file list is wrong, so this check proved nothing."
|
else
|
||||||
elif [ "$fails" = "$before" ]; then
|
# shellcheck disable=SC2086
|
||||||
echo " $guard_scanned shipped PHP files examined, every one guarded"
|
ci_php ci/check-abspath.php "--min=$MIN_SHIPPED_PHP_FILES" $shipped_php || fails=$((fails + 1))
|
||||||
fi
|
fi
|
||||||
|
|
||||||
ci_step "no debug leftovers"
|
ci_step "no debug leftovers"
|
||||||
|
|||||||
Reference in New Issue
Block a user