Code Coverage
 
Lines
Functions and Methods
Classes and Traits
Total
0.00% covered (danger)
0.00%
0 / 589
0.00% covered (danger)
0.00%
0 / 21
CRAP
0.00% covered (danger)
0.00%
0 / 1
MWVisitor
0.00% covered (danger)
0.00%
0 / 589
0.00% covered (danger)
0.00%
0 / 21
38220
0.00% covered (danger)
0.00%
0 / 1
 analyzeCallNode
0.00% covered (danger)
0.00%
0 / 23
0.00% covered (danger)
0.00%
0 / 1
110
 checkExternalLink
0.00% covered (danger)
0.00%
0 / 9
0.00% covered (danger)
0.00%
0 / 1
12
 doSelectWrapperSpecialHandling
0.00% covered (danger)
0.00%
0 / 48
0.00% covered (danger)
0.00%
0 / 1
420
 maybeTriggerHook
0.00% covered (danger)
0.00%
0 / 42
0.00% covered (danger)
0.00%
0 / 1
156
 hasPassByReferenceParameter
0.00% covered (danger)
0.00%
0 / 4
0.00% covered (danger)
0.00%
0 / 1
12
 getHookTypeForRegistrationMethod
0.00% covered (danger)
0.00%
0 / 6
0.00% covered (danger)
0.00%
0 / 1
20
 handleNormalHookRegistration
0.00% covered (danger)
0.00%
0 / 13
0.00% covered (danger)
0.00%
0 / 1
20
 handleParserHookRegistration
0.00% covered (danger)
0.00%
0 / 6
0.00% covered (danger)
0.00%
0 / 1
12
 registerHook
0.00% covered (danger)
0.00%
0 / 4
0.00% covered (danger)
0.00%
0 / 1
6
 visitReturn
0.00% covered (danger)
0.00%
0 / 22
0.00% covered (danger)
0.00%
0 / 1
56
 handleGetQueryInfoReturn
0.00% covered (danger)
0.00%
0 / 34
0.00% covered (danger)
0.00%
0 / 1
110
 checkMakeList
0.00% covered (danger)
0.00%
0 / 44
0.00% covered (danger)
0.00%
0 / 1
156
 literalListConstToName
0.00% covered (danger)
0.00%
0 / 15
0.00% covered (danger)
0.00%
0 / 1
56
 checkSQLOptions
0.00% covered (danger)
0.00%
0 / 10
0.00% covered (danger)
0.00%
0 / 1
30
 checkSQLOption
0.00% covered (danger)
0.00%
0 / 24
0.00% covered (danger)
0.00%
0 / 1
56
 checkJoinConds
0.00% covered (danger)
0.00%
0 / 48
0.00% covered (danger)
0.00%
0 / 1
272
 visitReturnOfFunctionHook
0.00% covered (danger)
0.00%
0 / 27
0.00% covered (danger)
0.00%
0 / 1
56
 getCallableFromHookRegistration
0.00% covered (danger)
0.00%
0 / 13
0.00% covered (danger)
0.00%
0 / 1
30
 visitAssign
0.00% covered (danger)
0.00%
0 / 27
0.00% covered (danger)
0.00%
0 / 1
272
 detectHTMLForm
0.00% covered (danger)
0.00%
0 / 167
0.00% covered (danger)
0.00%
0 / 1
1640
 visitArray
0.00% covered (danger)
0.00%
0 / 3
0.00% covered (danger)
0.00%
0 / 1
6
1<?php declare( strict_types = 1 );
2
3/**
4 * @license GPL-2.0-or-later
5 */
6namespace SecurityCheckPlugin;
7
8use ast\Node;
9use Phan\Analysis\PostOrderAnalysisVisitor;
10use Phan\AST\ContextNode;
11use Phan\AST\UnionTypeVisitor;
12use Phan\Debug;
13use Phan\Exception\CodeBaseException;
14use Phan\Exception\InvalidFQSENException;
15use Phan\Exception\IssueException;
16use Phan\Language\Element\ClassAliasRecord;
17use Phan\Language\Element\FunctionInterface;
18use Phan\Language\Element\Method;
19use Phan\Language\FQSEN\FullyQualifiedClassName;
20use Phan\Language\FQSEN\FullyQualifiedFunctionLikeName;
21use Phan\Language\FQSEN\FullyQualifiedFunctionName;
22use Phan\Language\FQSEN\FullyQualifiedMethodName;
23use Phan\Language\Type;
24use Phan\Language\Type\BoolType;
25use Phan\Language\Type\TrueType;
26use Phan\Language\UnionType;
27use UnexpectedValueException;
28use const ast\AST_METHOD_CALL;
29
30/**
31 * MediaWiki specific node visitor
32 */
33class MWVisitor extends TaintednessVisitor {
34    /**
35     * @todo This is a temporary hack. Proper solution is refactoring/avoiding overrideContext
36     * @var bool|null
37     * @suppress PhanWriteOnlyProtectedProperty
38     */
39    protected $isHook;
40
41    /**
42     * Try and recognize hook registration
43     * @inheritDoc
44     */
45    protected function analyzeCallNode( Node $node, iterable $funcs ): void {
46        parent::analyzeCallNode( $node, $funcs );
47        if ( !isset( $node->children['method'] ) ) {
48            // Called by visitCall
49            return;
50        }
51
52        assert( is_array( $funcs ) && count( $funcs ) === 1 );
53        $method = $funcs[0];
54        assert( $method instanceof Method );
55
56        // Should this be getDefiningFQSEN() instead?
57        $methodName = (string)$method->getFQSEN();
58        // $this->debug( __METHOD__, "Checking to see if we should register $methodName" );
59        switch ( $methodName ) {
60            case "\\MediaWiki\\Parser\\Parser::setFunctionHook":
61            case "\\MediaWiki\\Parser\\Parser::setHook":
62                $type = $this->getHookTypeForRegistrationMethod( $methodName );
63                if ( $type === null ) {
64                    break;
65                }
66                // $this->debug( __METHOD__, "registering $methodName as $type" );
67                $this->handleParserHookRegistration( $node, $type );
68                break;
69            case '\MediaWiki\HookContainer\HookContainer::register':
70                $this->handleNormalHookRegistration( $node );
71                break;
72            case '\MediaWiki\Linker\Linker::makeExternalLink':
73                $this->checkExternalLink( $node );
74                break;
75            default:
76                if ( str_starts_with( $method->getName(), 'on' ) ) {
77                    $this->maybeTriggerHook( $node, $method );
78                }
79                $this->doSelectWrapperSpecialHandling( $node, $method );
80        }
81    }
82
83    /**
84     * MediaWiki\Linker\Linker::makeExternalLink escaping depends on third argument
85     */
86    private function checkExternalLink( Node $node ): void {
87        $escapeArg = $this->resolveValue( $node->children['args']->children[2] ?? true );
88        $text = $node->children['args']->children[1] ?? null;
89        if ( !$escapeArg && $text instanceof Node ) {
90            $this->maybeEmitIssueSimplified(
91                new Taintedness( SecurityCheckPlugin::HTML_EXEC_TAINT ),
92                $text,
93                "Calling Linker::makeExternalLink with user controlled text " .
94                "and third argument set to false"
95            );
96        }
97    }
98
99    /**
100     * Special casing for complex format of IReadableDatabase::select
101     *
102     * This handles the $options, and $join_cond. Other args are
103     * handled through normal means
104     *
105     * @param Node $node Either an AST_METHOD_CALL or AST_STATIC_CALL
106     * @param Method $method
107     */
108    private function doSelectWrapperSpecialHandling( Node $node, Method $method ): void {
109        static $relevantMethods;
110        if ( !$relevantMethods ) {
111            $makeFQSEN = [ FullyQualifiedClassName::class, 'fromFullyQualifiedString' ];
112            $relevantMethods = [
113                'makeList' => $makeFQSEN( '\\Wikimedia\\Rdbms\\Platform\\ISQLPlatform' ),
114                'select' => $makeFQSEN( '\\Wikimedia\\Rdbms\\IReadableDatabase' ),
115                'selectField' => $makeFQSEN( '\\Wikimedia\\Rdbms\\IReadableDatabase' ),
116                'selectFieldValues' => $makeFQSEN( '\\Wikimedia\\Rdbms\\IReadableDatabase' ),
117                'selectSQLText' => $makeFQSEN( '\\Wikimedia\\Rdbms\\Platform\\ISQLPlatform' ),
118                'selectRowCount' => $makeFQSEN( '\\Wikimedia\\Rdbms\\IReadableDatabase' ),
119                'selectRow' => $makeFQSEN( '\\Wikimedia\\Rdbms\\IReadableDatabase' ),
120                'option' => $makeFQSEN( '\\Wikimedia\\Rdbms\\SelectQueryBuilder' ),
121                'options' => $makeFQSEN( '\\Wikimedia\\Rdbms\\SelectQueryBuilder' ),
122                'joinConds' => $makeFQSEN( '\\Wikimedia\\Rdbms\\SelectQueryBuilder' ),
123            ];
124        }
125
126        $name = $method->getName();
127        if ( !isset( $relevantMethods[$name] ) ) {
128            return;
129        }
130
131        if ( !self::isSubclassOf( $method->getClassFQSEN(), $relevantMethods[$name], $this->code_base ) ) {
132            return;
133        }
134
135        $args = $node->children['args']->children;
136        switch ( $name ) {
137            case 'select':
138            case 'selectField':
139            case 'selectFieldValues':
140            case 'selectSQLText':
141            case 'selectRowCount':
142            case 'selectRow':
143                if ( isset( $args[4] ) ) {
144                    $this->checkSQLOptions( $args[4] );
145                }
146                if ( isset( $args[5] ) ) {
147                    $this->checkJoinConds( $args[5] );
148                }
149                return;
150            case 'option':
151                if ( count( $args ) >= 2 ) {
152                    $this->checkSQLOption( $args[0], $args[1], $node );
153                }
154                return;
155            case 'options':
156                if ( $args ) {
157                    $this->checkSQLOptions( $args[0] );
158                }
159                return;
160            case 'joinConds':
161                if ( $args ) {
162                    $this->checkJoinConds( $args[0] );
163                }
164                return;
165            case 'makeList':
166                $this->checkMakeList( $node );
167                return;
168            default:
169                throw new UnexpectedValueException( "Should be unreachable, got $name" );
170        }
171    }
172
173    /**
174     * Check if we are running a hook (i.e., calling a hook method on a HookRunner interface).
175     */
176    private function maybeTriggerHook( Node $node, FunctionInterface $method ): void {
177        if ( $node->kind !== AST_METHOD_CALL || !$method instanceof Method ) {
178            return;
179        }
180
181        try {
182            $implementedInterfaces = $method->getClass( $this->code_base )->getInterfaceFQSENList();
183        } catch ( CodeBaseException $e ) {
184            $this->debug( __METHOD__, "Class not found for method $method" . $this->getDebugInfo( $e ) );
185            return;
186        }
187
188        // We assume that for a hook called Foo, the interface is called FooHook and the handler onFoo, and that the
189        // hook runner (but not necessarily the handler) implements the hook interface.
190        $hookInterfaceName = preg_replace( '/^on/', '', $method->getName() ) . 'Hook';
191        $foundHookInterface = false;
192        foreach ( $implementedInterfaces as $implementedInterfaceFQSEN ) {
193            if ( $implementedInterfaceFQSEN->getName() === $hookInterfaceName ) {
194                $foundHookInterface = true;
195                break;
196            }
197        }
198
199        if ( !$foundHookInterface ) {
200            return;
201        }
202
203        $args = $node->children['args']->children;
204
205        $hasPassByRef = self::hasPassByReferenceParameter( $method );
206        $analyzer = new PostOrderAnalysisVisitor( $this->code_base, $this->context, [] );
207        $argumentTypes = array_fill( 0, count( $args ), UnionType::empty() );
208
209        $subscribers = MediaWikiHooksHelper::getInstance()->getHookSubscribers( $method->getName() );
210        foreach ( $subscribers as $subscriber ) {
211            if ( $subscriber instanceof FullyQualifiedMethodName ) {
212                if ( !$this->code_base->hasMethodWithFQSEN( $subscriber ) ) {
213                    $this->debug( __METHOD__, "Hook subscriber $subscriber not found!" );
214                    continue;
215                }
216                $func = $this->code_base->getMethodByFQSEN( $subscriber );
217            } else {
218                assert( $subscriber instanceof FullyQualifiedFunctionName );
219                if ( !$this->code_base->hasFunctionWithFQSEN( $subscriber ) ) {
220                    $this->debug( __METHOD__, "Hook subscriber $subscriber not found!" );
221                    continue;
222                }
223                $func = $this->code_base->getFunctionByFQSEN( $subscriber );
224            }
225
226            // $this->debug( __METHOD__, "Dispatching $hookName to $subscriber" );
227            // This is hacky, but try to ensure that the associated line
228            // number for any issues is in the extension, and not the
229            // line where the HookContainer::register() is in MW core.
230            // FIXME: In the case of reference parameters, this is
231            // still reporting things being in MW core instead of extension.
232            $oldContext = $this->overrideContext;
233            $fContext = $func->getContext();
234            $newContext = clone $this->context;
235            $newContext = $newContext->withFile( $fContext->getFile() )
236                ->withLineNumberStart( $fContext->getLineNumberStart() );
237            $this->overrideContext = $newContext;
238            $this->isHook = true;
239
240            if ( $hasPassByRef ) {
241                // Trigger an analysis of the function call (see e.g. ClosureReturnTypeOverridePlugin's
242                // handling of call_user_func_array). Note that it's not enough to use our
243                // handleMethodCall, because that doesn't handle references correctly.
244
245                // NOTE: This is only known to be necessary with references, hence the check above
246                // (for performance). There might be other edge cases, though...
247
248                // TODO We don't care about types, so we use an empty union type. However this looks
249                // very very fragile.
250                // TODO 2: Someday we could write a generic-purpose MW plugin, which could (among other
251                // things) understand hook. It could share some code with taint-check, and at that
252                // point we'd likely want to use the correct types here.
253                $analyzer->analyzeCallableWithArgumentTypes( $argumentTypes, $func, $args );
254            }
255            $this->handleMethodCall( $func, $subscriber, $args, false, true );
256
257            $this->overrideContext = $oldContext;
258            $this->isHook = false;
259        }
260    }
261
262    /**
263     * Check whether a function takes a parameter by reference (copy of
264     * {@link \Phan\Language\Element\FunctionTrait::hasPassByReferenceVariable()})
265     */
266    private static function hasPassByReferenceParameter( FunctionInterface $func ): bool {
267        foreach ( $func->getParameterList() as $param ) {
268            if ( $param->isPassByReference() ) {
269                return true;
270            }
271        }
272        return false;
273    }
274
275    /**
276     * @param string $method The method name of the registration function
277     * @return string|null The name of the hook that gets registered
278     */
279    private function getHookTypeForRegistrationMethod( string $method ): ?string {
280        switch ( $method ) {
281            case "\\MediaWiki\\Parser\\Parser::setFunctionHook":
282                return '!ParserFunctionHook';
283            case "\\MediaWiki\\Parser\\Parser::setHook":
284                return '!ParserHook';
285            default:
286                $this->debug( __METHOD__, "$method not a hook registerer" );
287                return null;
288        }
289    }
290
291    /**
292     * Handle registering a normal hook from HookContainer::register (Not from $wgHooks)
293     *
294     * @param Node $node The node representing the AST_METHOD_CALL
295     */
296    private function handleNormalHookRegistration( Node $node ): void {
297        assert( $node->kind === \ast\AST_METHOD_CALL );
298        $params = $node->children['args']->children;
299        if ( count( $params ) < 2 ) {
300            $this->debug( __METHOD__, "Could not understand HookContainer::register" );
301            return;
302        }
303        $hookName = $params[0];
304        if ( !is_string( $hookName ) ) {
305            $this->debug( __METHOD__, "Could not register hook. Name is complex" );
306            return;
307        }
308        $cb = $this->getCallableFromHookRegistration( $params[1], $hookName );
309        if ( $cb ) {
310            $this->registerHook( $hookName, $cb );
311        } else {
312            $this->debug( __METHOD__, "Could not register $hookName hook due to complex callback" );
313        }
314    }
315
316    /**
317     * When someone calls $parser->setFunctionHook() or setTagHook()
318     *
319     * @note Causes phan to error out if given non-existent class
320     * @param Node $node The AST_METHOD_CALL node
321     * @param string $hookType The name of the hook
322     */
323    private function handleParserHookRegistration( Node $node, string $hookType ): void {
324        $args = $node->children['args']->children;
325        if ( count( $args ) < 2 ) {
326            return;
327        }
328        $callback = $this->getCallableFromNode( $args[1] );
329        if ( $callback ) {
330            $this->registerHook( $hookType, $callback );
331        }
332    }
333
334    private function registerHook( string $hookType, FunctionInterface $callback ): void {
335        $fqsen = $callback->getFQSEN();
336        $alreadyRegistered = MediaWikiHooksHelper::getInstance()->registerHook( $hookType, $fqsen );
337        if ( !$alreadyRegistered ) {
338            // $this->debug( __METHOD__, "registering $fqsen for hook $hookType" );
339            // If this is the first time seeing this, make sure we reanalyze the hook function now that
340            // we know what it is, in case it's already been analyzed.
341            $this->analyzeFunc( $callback );
342        }
343    }
344
345    /**
346     * For special hooks, check their return value
347     *
348     * e.g. A tag hook's return value is output as html.
349     */
350    public function visitReturn( Node $node ): void {
351        parent::visitReturn( $node );
352        if (
353            !$node->children['expr'] instanceof Node ||
354            !$this->context->isInFunctionLikeScope()
355        ) {
356            return;
357        }
358        $funcFQSEN = $this->context->getFunctionLikeFQSEN();
359        $funcFQSENStr = $funcFQSEN->__toString();
360
361        if (
362            // The one in SelectQueryBuilder is generic, don't bother. The stuff it returns is analyzed separately when
363            // we find calls to SelectQueryBuilder methods.
364            $funcFQSENStr !== '\\Wikimedia\\Rdbms\\SelectQueryBuilder::getQueryInfo' &&
365            str_ends_with( $funcFQSENStr, '::getQueryInfo' )
366        ) {
367            $this->handleGetQueryInfoReturn( $node->children['expr'] );
368        }
369
370        $hookType = MediaWikiHooksHelper::getInstance()->isSpecialHookSubscriber( $funcFQSEN );
371        switch ( $hookType ) {
372            case '!ParserFunctionHook':
373                $this->visitReturnOfFunctionHook( $node->children['expr'], $funcFQSEN );
374                break;
375            case '!ParserHook':
376                $ret = $node->children['expr'];
377                $this->maybeEmitIssueSimplified(
378                    new Taintedness( SecurityCheckPlugin::HTML_EXEC_TAINT ),
379                    $ret,
380                    "Outputting user controlled HTML from Parser tag hook {FUNCTIONLIKE}",
381                    [ $funcFQSEN ]
382                );
383                break;
384        }
385    }
386
387    /**
388     * Methods named getQueryInfo() in MediaWiki usually
389     * return an array that is later fed to select
390     *
391     * @note This will only work where the return
392     *  statement is an array literal.
393     * @param Node|mixed $node Node from ast tree
394     */
395    private function handleGetQueryInfoReturn( mixed $node ): void {
396        if (
397            !( $node instanceof Node ) ||
398            $node->kind !== \ast\AST_ARRAY
399        ) {
400            return;
401        }
402        // The argument order is
403        // $table, $vars, $conds = '', $fname = __METHOD__,
404        // $options = [], $join_conds = []
405        $keysToArg = [
406            'tables' => 0,
407            'fields' => 1,
408            'conds' => 2,
409            'options' => 4,
410            'join_conds' => 5,
411        ];
412        $args = [ '', '', '', '' ];
413        foreach ( $node->children as $child ) {
414            // Can't have array destructuring in a return statement.
415            assert( $child !== null );
416            if ( $child->kind === \ast\AST_UNPACK ) {
417                // Can't analyze this, skip it.
418                continue;
419            }
420            assert( $child->kind === \ast\AST_ARRAY_ELEM );
421            $key = $child->children['key'];
422            if ( $key instanceof Node ) {
423                // Dynamic name, skip (T268055).
424                continue;
425            }
426            if ( !isset( $keysToArg[$key] ) ) {
427                continue;
428            }
429            $args[$keysToArg[$key]] = $child->children['value'];
430        }
431        $selectFQSEN = FullyQualifiedMethodName::fromFullyQualifiedString(
432            '\Wikimedia\Rdbms\IReadableDatabase::select'
433        );
434        if ( !$this->code_base->hasMethodWithFQSEN( $selectFQSEN ) ) {
435            // Huh. Core wasn't parsed. That's bad, but don't fail hard.
436            $this->debug( __METHOD__, 'Database::select does not exist.' );
437            return;
438        }
439        $select = $this->code_base->getMethodByFQSEN( $selectFQSEN );
440        // TODO: The message about calling Database::select here is not very clear.
441        $this->handleMethodCall( $select, $selectFQSEN, $args, false );
442        if ( isset( $args[4] ) ) {
443            $this->checkSQLOptions( $args[4] );
444        }
445        if ( isset( $args[5] ) ) {
446            $this->checkJoinConds( $args[5] );
447        }
448    }
449
450    /**
451     * Check IDatabase::makeList
452     *
453     * Special cased because the second arg totally changes
454     * how this function is interpreted.
455     */
456    private function checkMakeList( Node $node ): void {
457        $args = $node->children['args'];
458        // First determine which IDatabase::LIST_*
459        // 0 = IDatabase::LIST_COMMA is default value.
460        $typeArg = $args->children[1] ?? 0;
461        if ( $typeArg instanceof Node ) {
462            $typeArg = $this->getCtxN( $typeArg )->getEquivalentPHPValueForNode(
463                $typeArg,
464                ContextNode::RESOLVE_SCALAR_DEFAULT & ~ContextNode::RESOLVE_CONSTANTS
465            );
466        }
467        if ( $typeArg instanceof Node ) {
468            if ( $typeArg->kind === \ast\AST_CLASS_CONST ) {
469                // Probably IDatabase::LIST_*. Note that non-class constants are resolved
470                $typeArg = $typeArg->children['const'];
471            } elseif ( $typeArg->kind === \ast\AST_CONST ) {
472                $typeArg = $typeArg->children['name']->children['name'];
473            } else {
474                // Something that cannot be resolved statically. Since LIST_NAMES is very rare, and LIST_COMMA is
475                // default, assume its LIST_AND or LIST_OR
476                $this->debug( __METHOD__, "Could not determine 2nd arg makeList()" );
477                $this->maybeEmitIssueSimplified(
478                    new Taintedness( SecurityCheckPlugin::SQL_NUMKEY_EXEC_TAINT ),
479                    $args->children[0],
480                    "IDatabase::makeList with unknown type arg is " .
481                    "given an array with unescaped keynames or " .
482                    "values for numeric keys (May be false positive)"
483                );
484
485                return;
486            }
487        }
488
489        // Make sure not to mix strings and ints in switch cases, as that will break horribly
490        if ( is_int( $typeArg ) ) {
491            $typeArg = $this->literalListConstToName( $typeArg );
492        }
493        switch ( $typeArg ) {
494            case 'LIST_COMMA':
495                // String keys ignored. Everything escaped. So nothing to worry about.
496                break;
497            case 'LIST_AND':
498            case 'LIST_SET':
499            case 'LIST_OR':
500                // exec_sql_numkey
501                $this->maybeEmitIssueSimplified(
502                    new Taintedness( SecurityCheckPlugin::SQL_NUMKEY_EXEC_TAINT ),
503                    $args->children[0],
504                    "IDatabase::makeList with LIST_AND, LIST_OR or "
505                    . "LIST_SET must sql escape string key names and values of numeric keys"
506                );
507                break;
508            case 'LIST_NAMES':
509                // Like comma but with no escaping.
510                $this->maybeEmitIssueSimplified(
511                    new Taintedness( SecurityCheckPlugin::SQL_EXEC_TAINT ),
512                    $args->children[0],
513                    "IDatabase::makeList with LIST_NAMES needs "
514                    . "to escape for SQL"
515                );
516                break;
517            default:
518                $this->debug( __METHOD__, "Unrecognized 2nd arg " . "to IDatabase::makeList: '$typeArg'" );
519        }
520    }
521
522    /**
523     * Convert a literal int value for a LIST_* constant to its name. This is a horrible hack for crappy code
524     * that uses the constants literally rather than by name. Such code shouldn't deserve taint analysis.
525     * This method can obviously break very easily if the values are changed.
526     */
527    private function literalListConstToName( int $value ): string {
528        switch ( $value ) {
529            case 0:
530                return 'LIST_COMMA';
531            case 1:
532                return 'LIST_AND';
533            case 2:
534                return 'LIST_SET';
535            case 3:
536                return 'LIST_NAMES';
537            case 4:
538                return 'LIST_OR';
539            default:
540                // Oh boy, what the heck are you doing? Well, DWIM
541                $this->debug(
542                    __METHOD__,
543                    'Someone specified a LIST_* constant literally but it is not a valid value. Wow.'
544                );
545                return 'LIST_AND';
546        }
547    }
548
549    /**
550     * Check the options parameter to IReadableDatabase::select
551     *
552     * This only works if its specified as an array literal.
553     *
554     * @param Node|mixed $node The node from the AST tree
555     */
556    private function checkSQLOptions( mixed $node ): void {
557        if ( !( $node instanceof Node ) || $node->kind !== \ast\AST_ARRAY ) {
558            return;
559        }
560
561        foreach ( $node->children as $arrayElm ) {
562            // Can't use array destructuring as an expression
563            assert( $arrayElm !== null );
564            if ( $arrayElm->kind === \ast\AST_UNPACK ) {
565                // Can't analyze this, skip it.
566                continue;
567            }
568            assert( $arrayElm->kind === \ast\AST_ARRAY_ELEM );
569            $val = $arrayElm->children['value'];
570            $key = $arrayElm->children['key'];
571            $this->checkSQLOption( $key, $val, $node );
572        }
573    }
574
575    /**
576     *  Relevant options:
577     *   GROUP BY is put directly in the query (array gets imploded)
578     *   HAVING is treated like a WHERE clause
579     *   ORDER BY is put directly in the query (array gets imploded)
580     *   USE INDEX is directly put in string (both array and string version)
581     *   IGNORE INDEX ditto
582     *
583     * @param Node|mixed $option
584     * @param Node|mixed $value
585     * @param Node $callNode
586     */
587    private function checkSQLOption( mixed $option, mixed $value, Node $callNode ): void {
588        $relevant = [
589            'GROUP BY' => true,
590            'ORDER BY' => true,
591            'HAVING' => true,
592            'USE INDEX' => true,
593            'IGNORE INDEX' => true,
594        ];
595
596        if ( !is_string( $option ) || !isset( $relevant[$option] ) ) {
597            return;
598        }
599        $taintType = ( $option === 'HAVING' && $this->nodeIsArray( $value ) ) ?
600            SecurityCheckPlugin::SQL_NUMKEY_EXEC_TAINT :
601            SecurityCheckPlugin::SQL_EXEC_TAINT;
602        $taintType = new Taintedness( $taintType );
603
604        $this->backpropagateArgTaint( $callNode, $taintType );
605        if ( $value instanceof Node && $value->lineno !== $this->context->getLineNumberStart() ) {
606            $ctx = clone $this->context;
607            $this->overrideContext = $ctx->withLineNumberStart( $value->lineno );
608        }
609        $this->maybeEmitIssueSimplified(
610            $taintType,
611            $value,
612            "{STRING_LITERAL} clause is user controlled",
613            [ $option ]
614        );
615        $this->overrideContext = null;
616    }
617
618    /**
619     * Check a join_cond structure.
620     *
621     * Syntax is like
622     *
623     *  [ 'aliasOfTable' => [ 'JOIN TYPE', $onConditions ], ... ]
624     *  join type is usually something safe like INNER JOIN, but it is not
625     *  validated or escaped. $onConditions is the same form as a WHERE clause.
626     *
627     * @param Node|mixed $node
628     */
629    private function checkJoinConds( mixed $node ): void {
630        if ( !( $node instanceof Node ) || $node->kind !== \ast\AST_ARRAY ) {
631            return;
632        }
633
634        foreach ( $node->children as $table ) {
635            // Can't use array destructuring as an expression
636            assert( $table !== null );
637            if ( $table->kind === \ast\AST_UNPACK ) {
638                // Can't analyze this, skip it.
639                continue;
640            }
641            assert( $table->kind === \ast\AST_ARRAY_ELEM );
642
643            $tableName = is_string( $table->children['key'] ) ?
644                $table->children['key'] :
645                '[UNKNOWN TABLE]';
646            $joinInfo = $table->children['value'];
647            if ( $joinInfo instanceof Node && $joinInfo->kind === \ast\AST_ARRAY ) {
648                if (
649                    count( $joinInfo->children ) === 0 ||
650                    $joinInfo->children[0]->children['key'] !== null
651                ) {
652                    $this->debug( __METHOD__, "join info has named key??" );
653                    continue;
654                }
655                $joinType = $joinInfo->children[0]->children['value'];
656                // join type does not get escaped.
657                $this->maybeEmitIssueSimplified(
658                    new Taintedness( SecurityCheckPlugin::SQL_EXEC_TAINT ),
659                    $joinType,
660                    "Join type for {STRING_LITERAL} is user controlled",
661                    [ $tableName ]
662                );
663                if ( $joinType instanceof Node ) {
664                    $this->backpropagateArgTaint(
665                        $joinType,
666                        new Taintedness( SecurityCheckPlugin::SQL_EXEC_TAINT )
667                    );
668                }
669                // On to the join ON conditions.
670                if (
671                    count( $joinInfo->children ) === 1 ||
672                    $joinInfo->children[1]->children['key'] !== null
673                ) {
674                    $this->debug( __METHOD__, "join info has named key??" );
675                    continue;
676                }
677                $onCond = $joinInfo->children[1]->children['value'];
678                if ( $onCond instanceof Node && $onCond->lineno !== $this->context->getLineNumberStart() ) {
679                    $ctx = clone $this->context;
680                    $this->overrideContext = $ctx->withLineNumberStart( $onCond->lineno );
681                }
682                $this->maybeEmitIssueSimplified(
683                    new Taintedness( SecurityCheckPlugin::SQL_NUMKEY_EXEC_TAINT ),
684                    $onCond,
685                    "The ON conditions are not properly escaped for the join to `{STRING_LITERAL}`",
686                    [ $tableName ]
687                );
688                if ( $onCond instanceof Node ) {
689                    $this->backpropagateArgTaint(
690                        $onCond,
691                        new Taintedness( SecurityCheckPlugin::SQL_NUMKEY_EXEC_TAINT )
692                    );
693                }
694                $this->overrideContext = null;
695            }
696        }
697    }
698
699    /**
700     * Check to see if isHTML => true and is tainted.
701     *
702     * @param Node $node The expr child of the return. NOT the return itself
703     * @param FullyQualifiedFunctionLikeName $funcName
704     */
705    private function visitReturnOfFunctionHook( Node $node, FullyQualifiedFunctionLikeName $funcName ): void {
706        // XXX: we limit this to literal arrays because otherwise, we wouldn't be able to match the output taintedness
707        // and the `isHTML` value from different branches. See `safeHookIndirect1` test.
708        if ( $node->kind !== \ast\AST_ARRAY || count( $node->children ) < 2 ) {
709            return;
710        }
711        $arg = $node->children[0];
712        // Can't have array destructuring in a return statement.
713        assert( $arg instanceof Node );
714        if ( $arg->kind === \ast\AST_UNPACK ) {
715            // Can't analyze this.
716            return;
717        }
718        assert( $arg->kind === \ast\AST_ARRAY_ELEM );
719
720        $retType = UnionTypeVisitor::unionTypeFromNode( $this->code_base, $this->context, $node );
721        $isHTMLElementType = UnionTypeVisitor::resolveArrayShapeElementTypesForOffset(
722            $retType,
723            'isHTML',
724            false,
725            $this->code_base
726        );
727        if ( !$isHTMLElementType instanceof UnionType ) {
728            // Can't be resolved statically.
729            return;
730        }
731
732        // NOTE: Cannot use `containsTrue` here, because it, huh, also returns true for `false`.
733        $containsTrue = $isHTMLElementType->hasRealTypeMatchingCallback(
734            static fn ( Type $t ): bool => $t instanceof BoolType || $t instanceof TrueType
735        );
736        if ( !$containsTrue ) {
737            return;
738        }
739
740        $this->maybeEmitIssueSimplified(
741            new Taintedness( SecurityCheckPlugin::HTML_EXEC_TAINT ),
742            $arg->children['value'],
743            "Outputting user controlled HTML from Parser function hook {FUNCTIONLIKE}",
744            [ $funcName ]
745        );
746    }
747
748    /**
749     * Given a MediaWiki hook registration, find the callback
750     *
751     * @note This is a different format than Parser hooks use.
752     *
753     * Valid examples of callbacks:
754     *  1) A normal callable (string, array, or first-class)
755     *  2) A class instance with an `on$hook` method
756     *  3) An extension hook handler spec (not handled here as we check extension.json instead)
757     *  4) `HookContainer::NOOP` for no-op handlers
758     *
759     * @param Node|mixed $node
760     * @param string $hookName
761     */
762    private function getCallableFromHookRegistration( mixed $node, string $hookName ): ?FunctionInterface {
763        $cb = $this->getCallableFromNode( $node );
764        if ( $cb ) {
765            return $cb;
766        }
767
768        $methodName = 'on' . $hookName;
769
770        try {
771            // Don't warn if it's the wrong type, for it might be a callable and not a class.
772            $classes = $this->getCtxN( $node )->getClassList( true, ContextNode::CLASS_LIST_ACCEPT_ANY, null, false );
773        } catch ( CodeBaseException | IssueException ) {
774            $classes = [];
775        }
776        foreach ( $classes as $class ) {
777            try {
778                return $class->getMethodByName( $this->code_base, $methodName );
779            } catch ( CodeBaseException ) {
780                continue;
781            }
782        }
783
784        // @todo Should probably emit a non-security issue
785        $this->debug( __METHOD__, "Missing hook handler for node: " . Debug::nodeToString( $node ) );
786        return null;
787    }
788
789    /**
790     * Check for $wgHooks registration
791     *
792     * @param Node $node
793     * @note This assumes $wgHooks is always the global
794     *   even if there is no globals declaration.
795     */
796    public function visitAssign( Node $node ): void {
797        parent::visitAssign( $node );
798
799        $var = $node->children['var'];
800        if ( !$var instanceof Node ) {
801            // Syntax error
802            return;
803        }
804        $hookName = null;
805        $expr = $node->children['expr'];
806        // The $wgHooks['foo'][] case
807        if (
808            $var->kind === \ast\AST_DIM &&
809            $var->children['dim'] === null &&
810            $var->children['expr'] instanceof Node &&
811            $var->children['expr']->kind === \ast\AST_DIM &&
812            $var->children['expr']->children['expr'] instanceof Node &&
813            is_string( $var->children['expr']->children['dim'] ) &&
814            /* The $wgHooks['SomeHook'][] case */
815            ( ( $var->children['expr']->children['expr']->kind === \ast\AST_VAR &&
816            $var->children['expr']->children['expr']->children['name'] === 'wgHooks' ) ||
817            /* The $GLOBALS['wgHooks']['SomeHook'][] case */
818            ( $var->children['expr']->children['expr']->kind === \ast\AST_DIM &&
819            $var->children['expr']->children['expr']->children['expr'] instanceof Node &&
820            $var->children['expr']->children['expr']->children['expr']->kind === \ast\AST_VAR &&
821            $var->children['expr']->children['expr']->children['expr']->children['name'] === 'GLOBALS' ) )
822        ) {
823            $hookName = $var->children['expr']->children['dim'];
824        }
825
826        if ( $hookName !== null ) {
827            $cb = $this->getCallableFromHookRegistration( $expr, $hookName );
828            if ( $cb ) {
829                $this->registerHook( $hookName, $cb );
830            } else {
831                $this->debug( __METHOD__, "Could not register hook " .
832                    "$hookName due to complex callback"
833                );
834            }
835        }
836    }
837
838    /**
839     * Special implementation of visitArray to detect HTMLForm specifiers
840     */
841    private function detectHTMLForm( Node $node ): void {
842        // Try to immediately filter out things that certainly aren't HTMLForms
843        $maybeHTMLForm = false;
844        foreach ( $node->children as $child ) {
845            if ( $child instanceof Node && $child->kind === \ast\AST_ARRAY_ELEM ) {
846                $key = $child->children['key'];
847                if ( $key instanceof Node || $key === 'class' || $key === 'type' ) {
848                    $maybeHTMLForm = true;
849                    break;
850                }
851            }
852        }
853        if ( !$maybeHTMLForm ) {
854            return;
855        }
856
857        $authReqFQSEN = FullyQualifiedClassName::fromFullyQualifiedString(
858            'MediaWiki\Auth\AuthenticationRequest'
859        );
860
861        if (
862            $this->code_base->hasClassWithFQSEN( $authReqFQSEN ) &&
863            $this->context->isInClassScope() &&
864            self::isSubclassOf( $this->context->getClassFQSEN(), $authReqFQSEN, $this->code_base )
865        ) {
866            // AuthenticationRequest::getFieldInfo() defines a very
867            // similar array but with different rules. T202112
868            return;
869        }
870
871        // This is a rather superficial check. There
872        // are many ways to construct htmlform specifiers this
873        // won't catch, and it may also have some false positives.
874
875        static $HTMLFormTypesToClasses = null;
876        if ( !$HTMLFormTypesToClasses ) {
877            $makeFQSEN = FullyQualifiedClassName::fromFullyQualifiedString( ... );
878            $HTMLFormTypesToClasses = [
879                'api' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLApiField' ),
880                'text' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTextField' ),
881                'textwithbutton' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTextFieldWithButton' ),
882                'textarea' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTextAreaField' ),
883                'select' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLSelectField' ),
884                'combobox' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLComboboxField' ),
885                'radio' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLRadioField' ),
886                'multiselect' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLMultiSelectField' ),
887                'limitselect' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLSelectLimitField' ),
888                'check' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLCheckField' ),
889                'toggle' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLCheckField' ),
890                'int' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLIntField' ),
891                'file' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLFileField' ),
892                'float' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLFloatField' ),
893                'info' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLInfoField' ),
894                'selectorother' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLSelectOrOtherField' ),
895                'selectandother' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLSelectAndOtherField' ),
896                'namespaceselect' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLSelectNamespace' ),
897                'namespaceselectwithbutton' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLSelectNamespaceWithButton' ),
898                'tagfilter' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTagFilter' ),
899                'sizefilter' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLSizeFilterField' ),
900                'submit' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLSubmitField' ),
901                'hidden' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLHiddenField' ),
902                'edittools' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLEditTools' ),
903                'checkmatrix' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLCheckMatrix' ),
904                'cloner' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLFormFieldCloner' ),
905                'autocompleteselect' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLAutoCompleteSelectField' ),
906                'language' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLSelectLanguageField' ),
907                'date' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLDateTimeField' ),
908                'time' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLDateTimeField' ),
909                'datetime' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLDateTimeField' ),
910                'expiry' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLExpiryField' ),
911                'timezone' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTimezoneField' ),
912                'email' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTextField' ),
913                'password' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTextField' ),
914                'url' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTextField' ),
915                'title' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTitleTextField' ),
916                'user' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLUserTextField' ),
917                'tagmultiselect' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTagMultiselectField' ),
918                'orderedmultiselect' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLOrderedMultiselectField' ),
919                'usersmultiselect' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLUsersMultiselectField' ),
920                'titlesmultiselect' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLTitlesMultiselectField' ),
921                'namespacesmultiselect' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLNamespacesMultiselectField' ),
922                // NOTE: it isn't actually possible to create an HTMLButtonField using `type => button` for some reason.
923                // Here, pretending it to be possible is simpler than special-casing the exception.
924                'button' => $makeFQSEN( '\MediaWiki\HTMLForm\Field\HTMLButtonField' ),
925            ];
926        }
927        static $rawProps = [
928            'label-raw',
929            'help-raw',
930            'buttonlabel-raw',
931        ];
932        static $propsToResolve = null;
933        $propsToResolve ??= [
934            'type',
935            'class',
936            'label',
937            'options',
938            'default',
939            'raw',
940            'rawrow',
941            // TODO: remove help key case when back compat is no longer needed (T356971)
942            'help',
943            ...$rawProps,
944        ];
945
946        $fieldProps = [];
947        foreach ( $node->children as $child ) {
948            if ( $child === null || $child->kind === \ast\AST_UNPACK ) {
949                // If we have list( , $x ) = foo(), or an in-place unpack, chances are this is not an HTMLForm.
950                return;
951            }
952            assert( $child->kind === \ast\AST_ARRAY_ELEM );
953            if ( $child->children['key'] === null ) {
954                // Implicit offset, hence most certainly not an HTMLForm.
955                return;
956            }
957            $key = $this->resolveOffset( $child->children['key'] );
958            if ( !is_string( $key ) ) {
959                // Either not resolvable (so nothing we can say) or a non-string literal, skip.
960                return;
961            }
962            if ( in_array( $key, $propsToResolve, true ) ) {
963                $fieldProps[$key] = $this->resolveValue( $child->children['value'] );
964            }
965        }
966        // Special case
967        $raw = $fieldProps['raw'] ?? $fieldProps['rawrow'] ?? null;
968
969        // Also important to reject empty string, not just
970        // null, otherwise 9e409c781015 of Wikibase causes
971        // this to fatal
972        if ( !empty( $fieldProps['type'] ) && is_string( $fieldProps['type'] ) ) {
973            $type = $fieldProps['type'];
974            if ( !isset( $HTMLFormTypesToClasses[$type] ) ) {
975                // Not a valid HTMLForm field (or a new field type we don't recognize)
976                return;
977            }
978        } elseif ( !empty( $fieldProps['class'] ) && is_string( $fieldProps['class'] ) ) {
979            try {
980                $fqsen = FullyQualifiedClassName::fromStringInContext(
981                    $fieldProps['class'],
982                    $this->context
983                );
984            } catch ( InvalidFQSENException ) {
985                // 'class' refers to something which is not a class, and this is probably not
986                // an HTMLForm
987                return;
988            }
989
990            $type = null;
991            foreach ( $HTMLFormTypesToClasses as $curType => $fieldFQSEN ) {
992                if ( $fqsen === $fieldFQSEN ) {
993                    $type = $curType;
994                    break;
995                }
996                // Note, this assumes that the list above uses the canonical FQSENs
997                $fieldAliasFQSENs = array_map(
998                    static fn ( ClassAliasRecord $car ): FullyQualifiedClassName => $car->alias_fqsen,
999                    $this->code_base->getClassAliasesByFQSEN( $fieldFQSEN )
1000                );
1001                if ( in_array( $fqsen, $fieldAliasFQSENs, true ) ) {
1002                    $type = $curType;
1003                    break;
1004                }
1005            }
1006            if ( !$type ) {
1007                // Not a valid HTMLForm field (or a new field type we don't recognize)
1008                return;
1009            }
1010        } else {
1011            // Definitely not an HTMLForm
1012            return;
1013        }
1014
1015        $fieldPropsToCheck = array_diff_key( $fieldProps, [ 'class' => 1, 'type' => 1 ] );
1016        if ( !$fieldPropsToCheck ) {
1017            // e.g. [ 'class' => 'someCssClass' ] appears a lot
1018            // in the code base. If we don't have any of the interesting
1019            // fields, skip out early.
1020            return;
1021        }
1022
1023        if ( $fieldPropsToCheck['label'] ?? null ) {
1024            // double escape check for label.
1025            $this->maybeEmitIssueSimplified(
1026                new Taintedness( SecurityCheckPlugin::ESCAPED_EXEC_TAINT ),
1027                $fieldPropsToCheck['label'],
1028                'HTMLForm label key escapes its input'
1029            );
1030        }
1031        if ( $fieldPropsToCheck['help'] ?? null ) {
1032            $this->maybeEmitIssueSimplified(
1033                new Taintedness( SecurityCheckPlugin::HTML_EXEC_TAINT ),
1034                $fieldPropsToCheck['help'],
1035                'HTMLForm help needs to escape input'
1036            );
1037        }
1038        foreach ( $rawProps as $prop ) {
1039            if ( $fieldPropsToCheck[$prop] ?? null ) {
1040                $this->maybeEmitIssueSimplified(
1041                    new Taintedness( SecurityCheckPlugin::HTML_EXEC_TAINT ),
1042                    $fieldPropsToCheck[$prop],
1043                    "HTMLForm $prop needs to escape input"
1044                );
1045            }
1046        }
1047
1048        if ( $type === 'info' && ( $fieldPropsToCheck['default'] ?? null ) ) {
1049            if ( $raw === true ) {
1050                $this->maybeEmitIssueSimplified(
1051                    new Taintedness( SecurityCheckPlugin::HTML_EXEC_TAINT ),
1052                    $fieldPropsToCheck['default'],
1053                    'HTMLForm info field in raw mode needs to escape default key'
1054                );
1055            }
1056            if ( $raw === false || $raw === null ) {
1057                $this->maybeEmitIssueSimplified(
1058                    new Taintedness( SecurityCheckPlugin::ESCAPED_EXEC_TAINT ),
1059                    $fieldPropsToCheck['default'],
1060                    'HTMLForm info field (non-raw) escapes default key already'
1061                );
1062            }
1063        }
1064
1065        // options key is really messed up with escaping.
1066        $isOptionsSafe = !in_array( $type, [ 'radio', 'multiselect' ], true );
1067        $options = $fieldProps['options'] ?? null;
1068        if ( !$isOptionsSafe && $options instanceof Node ) {
1069            $htmlExecTaint = new Taintedness( SecurityCheckPlugin::HTML_EXEC_TAINT );
1070            $optTaint = $this->getTaintedness( $options );
1071            $this->maybeEmitIssue(
1072                $htmlExecTaint,
1073                $optTaint->getTaintedness()->asKeyForForeach(),
1074                'HTMLForm option label needs escaping{DETAILS}',
1075                [ [ 'lines' => $optTaint->getError(), 'sink' => false ] ]
1076            );
1077        }
1078    }
1079
1080    /**
1081     * Try to detect HTMLForm specifiers
1082     */
1083    public function visitArray( Node $node ): void {
1084        parent::visitArray( $node );
1085        // Performance: use isset(), not property_exists
1086        if ( !isset( $node->skipHTMLFormAnalysis ) ) {
1087            $this->detectHTMLForm( $node );
1088        }
1089    }
1090}