plugins - arbitrary option value deletion risk

admin管理员组

文章数量:1406010

WP plugins is saying that one of our plugins is a security risk. From patchstack:

The plugin contains SQL Injection vulnerabilities due to unprepared queries: In delete_eto, the $id parameter is used unsafely in a delete operation.

Log in as an administrator.

GET /wp-admin/edit.php?post_type=xxx&page=xxx-templates&action=delete-single&gid=193&_wpnonce=3b166c400a

This delete all wp_options table records.

The plugin includes a screen in wp-admin that is only accessible by site-admins. On that screen, options can be created, edited and deleted.

This is the offensive function:

function delete_eto( $id ) {
    global $wpdb;
        
    $id = absint( $id );

    $wpdb->query( "DELETE FROM $wpdb->options WHERE option_id = $id" );

}

From the plugin team:

Passing the value of $id through absint() will not protect against arbitrary option value deletion.

The value of $id comes from GET, so the variable could be changed by a site-admin.

If I passed the option name in the GET and then used delete_option(), the option name could also be changed to an 'arbitrary option value'.

Is there a solution to this issue while using GET or should I convert to use POST ? Or do they just want me to use 'prepare' in the sql?

EDIT: The plugin team accepted this ( capability is checked earlier ) :

function delete_eto( $id ) {
    global $wpdb;
    
    $id = absint( $id );
    
    $option_name = $wpdb->get_var( "SELECT option_name FROM $wpdb->options WHERE option_id = $id" );
    
    if ( strncmp( $option_name, "xx-xxxx-", 8 ) === 0 ) {
        $wpdb->query( "DELETE FROM $wpdb->options WHERE option_id = $id" );
    }
    
}

WP plugins is saying that one of our plugins is a security risk. From patchstack:

The plugin contains SQL Injection vulnerabilities due to unprepared queries: In delete_eto, the $id parameter is used unsafely in a delete operation.

Log in as an administrator.

GET /wp-admin/edit.php?post_type=xxx&page=xxx-templates&action=delete-single&gid=193&_wpnonce=3b166c400a

This delete all wp_options table records.

The plugin includes a screen in wp-admin that is only accessible by site-admins. On that screen, options can be created, edited and deleted.

This is the offensive function:

function delete_eto( $id ) {
    global $wpdb;
        
    $id = absint( $id );

    $wpdb->query( "DELETE FROM $wpdb->options WHERE option_id = $id" );

}

From the plugin team:

Passing the value of $id through absint() will not protect against arbitrary option value deletion.

The value of $id comes from GET, so the variable could be changed by a site-admin.

If I passed the option name in the GET and then used delete_option(), the option name could also be changed to an 'arbitrary option value'.

Is there a solution to this issue while using GET or should I convert to use POST ? Or do they just want me to use 'prepare' in the sql?

EDIT: The plugin team accepted this ( capability is checked earlier ) :

function delete_eto( $id ) {
    global $wpdb;
    
    $id = absint( $id );
    
    $option_name = $wpdb->get_var( "SELECT option_name FROM $wpdb->options WHERE option_id = $id" );
    
    if ( strncmp( $option_name, "xx-xxxx-", 8 ) === 0 ) {
        $wpdb->query( "DELETE FROM $wpdb->options WHERE option_id = $id" );
    }
    
}

• plugins
• security

Share Improve this question edited Mar 14 at 14:54 shanebp asked Mar 6 at 23:56 shanebpshanebp 5,08577 gold badges2727 silver badges4040 bronze badges 2

• there is two parts here : the use of a prepared query and the test of the capabilities before to remove an option. – mmm Commented Mar 7 at 7:35
• and I don't understand why you don't use delete_option – mmm Commented Mar 7 at 7:37

Add a comment  | 

1 Answer 1

Sorted by: Reset to default 1

Yes, you can use GET with an option name. Just pass it through a validation function to verify that the name belongs to your codebase. Here is an example with both validation and capability check - it never harms to double-check that the user is allowed to perform the requested action.

function delete_eto( string $option ): void {
    if ( wpse_428642_should_delete_option( $option ) ) {
        delete_option( $option );
    }
}

function wpse_428642_should_delete_option( string $name ): bool {
    return wpse_428642_is_deletable_option( $name )
        && current_user_can( wpse_428642_options_required_capability() );
}

function wpse_428642_options_required_capability(): string {
    return 'manage_options';
}

function wpse_428642_is_deletable_option( string $name ): bool {
    return $name && in_array( $name, array(
        // 'option_foo',
        // 'option_bar',
        // ...
    ) );
}

本文标签： pluginsarbitrary option value deletion risk