Font Library: sanitize font collection data

[matiasbenedetto]

What?

Font Library: sanitize font collection data.

• Removes all the potential unwanted properties.
• Sanitizes all the values.
• Adds unit test case to ensure that data returned by get_data is always sanitized.

Why?

To return just the data that's safe to return.

How?

• Adding a data schema in the WP_Font_Collection class.
• Sanitizing the data using the sanitizing util from WP_Font_Utils

Testing Instructions

Run this PHP snippet featuring a font collection with risky data:

function register_collections() {
    $ubuntu = array(
        'font_family_settings' => array (
            'name'       => 'Ubuntu<script>alert("xss")</script>',
            'fontFamily' => 'Ubuntu, sans-serif<script>alert("xss")</script>',
            'slug'       => 'ubuntu<script>alert("xss")</script>',
        ),
        'categories' => array(
            'sans-serif',
        ),
    );
    
    $verdana = array(
        'font_family_settings' => array (
            'name'       => 'Verdana<script>alert("xss")</script>',
            'fontFamily' => 'Verdana, sans-serif<script>alert("xss")</script>',
            'slug'       => 'verdana<script>alert("xss")</script>',
        ),
        'categories' => array(
            'sans-serif',
        ),
    );
    
    $font_families = array ( $ubuntu, $verdana );
    
    $categories = array(
        array (
            'name' => 'Sans Serif<script>alert("xss")</script>',
            'slug' => 'sans-serif<script>alert("xss")</script>',
        ),
    );
    
    $collection_with_xss = array(
        'name'          => 'PHP Custom Collection',
        'description'   => __( 'Custom fonts collection' ),
        'font_families' => $font_families,
        'categories'    => $categories
    );
    
    wp_register_font_collection( 'collection-with-xss', $collection_with_xss );
}

add_action( 'rest_api_init', 'register_collections' );

Request that font collection using the API:

/wp-json/wp/v2/font-collections/collection-with-xss

the response should be:

{
    "slug": "collection-with-xss",
    "name": "PHP Custom Collection",
    "description": "Custom fonts collection",
    "font_families": [
        {
            "font_family_settings": {
                "name": "Ubuntu",
                "fontFamily": "Ubuntu, sans-serif",
                "slug": "ubuntualertxss"
            },
            "categories": [
                "sans-serif"
            ]
        },
        {
            "font_family_settings": {
                "name": "Verdana",
                "fontFamily": "Verdana, sans-serif",
                "slug": "verdanaalertxss"
            },
            "categories": [
                "sans-serif"
            ]
        }
    ],
    "categories": [
        {
            "name": "Sans Serif",
            "slug": "sans-serifalertxss"
        }
    ],
    "_links": {
        "self": [
            {
                "href": "http://localhost/wp1/wp-json/wp/v2/font-collections/collection-with-xss"
            }
        ],
        "collection": [
            {
                "href": "http://localhost/wp1/wp-json/wp/v2/font-collections"
            }
        ]
    }
}

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core SVN

If you're a Core Committer, use this list when committing to wordpress-develop in SVN:

Props: mmaattiiaass, grantmkin, youknowriad.

GitHub Merge commits

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: matiasbenedetto <[email protected]>
Co-authored-by: creativecoder <[email protected]>
Co-authored-by: youknowriad <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files

❔ lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php
❔ lib/compat/wordpress-6.5/fonts/class-wp-font-utils.php
❔ phpunit/tests/fonts/font-library/wpFontCollection/__construct.php
❔ phpunit/tests/fonts/font-library/wpFontCollection/getData.php
❔ phpunit/tests/fonts/font-library/wpFontLibrary/getFontCollection.php
❔ phpunit/tests/fonts/font-library/wpFontLibrary/registerFontCollection.php
❔ phpunit/tests/fonts/font-library/wpFontLibrary/unregisterFontCollection.php

[creativecoder]

@matiasbenedetto I made an attempt to streamline this and pushed a commit to this branch:

• Sanitize and validate as early as possible, so that appropriate notices are logged right away.
• Use static method for sanitization schema rather than a class constant so that we can use a closure for src sanitization rather than needing to add a new public method to the class.
• Adds a check within WP_Font_Utils::sanitize_from_schema so that class callables like array( $this, 'sanitization_method' ) can also be used
• Updates method name to sanitize_and_validate_data to indicate that sanitization is done first, as it might remove invalid data and affect the validation result.

Feel free to revert if you don't agree with the changes. Otherwise I think this now looks ready to go.

[creativecoder]

Adding this check allows using a class instance method, like array( $this, 'sanitization_method' ).