WordPress / WordPress/WordPress-Coding-Standards

Detect failures to use esc_attr() for printing in HTML attributes

Open
#527 6 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Component: Core Focus: Security Type: Enhancement
Dominant language
PHP
Stars
2.8k
Forks
521
Avg merge
5d 20h
Merged PRs (30d)
1

Description

The WordPress.XSS.EscapeOutput sniff will check if unescaped data is output. However, it does not distinguish between output contexts, meaning that the following is not flagged as a error when it obviously should be:

<?php $value = 'end of attribute" onclick="evil()'; ?>
<input type=button value="<?php echo esc_html( $value ) ?>">

The sniff should be updated to help guard against an HTML attribute injection vulnerability.

I suggest that the sniff be updated to look for instances of non-attribute escaping functions, and if the printing function was immediately preceded by a PHP open tag, which in turn is immediately preceded by a string that matches /\w+=['"]$/, that an error should be raised to indicate that esc_attr() should be used.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the WordPress.XSS.EscapeOutput sniff and trace how it detects output context around PHP and HTML. Reproduce the issue's input-attribute example and verify that non-attribute escaping is rejected and esc_attr() is required for the attribute value.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
security, tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.