CodeSOD: The Core Class

We've had a bit of a run of PHP in the inbox lately, which is fine, but certainly isn't doing anything to help PHP's reputation as a WTF factory. This one comes from Lucio C, who was hired to fix a hacked WordPress installation.

Much of the fixing was figuring out what data was safe to recover, what files may have been tampered with, and generally assessing the damage.

While doing that assessment, Lucio found this perfectly legitimate file in a perfectly legitimate WordPress plugin. This file was not altered by the hackers, but…

/** * The core class, where logic is defined. */ class Core { public $footer_content; // [...] public function footer_content() { if ( '' !== $this->footer_content && !is_admin() ) { $html = '<div id="igm-hidden-footer-content">' . $this->footer_content . '</div>'; // we should sanitize for security, but users //want to include all kinds of content, including forms. /* $allowed_html = wp_kses_allowed_html( 'post' ); $allowed_html['style'] = [ 'type' => true, ]; echo wp_kses( $html, $allowed_html ); */ echo $html ; } }

The opening comment is a wonderfully useless comment on what is clearly a "god class" object, where just all the plugin logic got dumped.

But it's this comment which highlights the WTF:

// we should sanitize for security, but users want to include all kinds of content, including forms.

"We probably shouldn't let this have an XSS vulnerability, but we can't stop it because our users don't care." There's even a vestigial attempt at doing some sanitization, commented out in favor of a version that just dumps user supplied HTML directly to the page.

What could go wrong?

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published. Required fields are marked *