Missing __() or _e() for translation

On line 1621, and 1832, and 1847, and 1867 you have:

echo "<p>Nice Try...</p>";

The above line, while untranslatable seems to be inside functions that duplicate existing WordPress access $capability checks and could therefore be removed altogether while moving the functionality to the fourth argument in add_submenu_page();

On line 1964 you have:

wp_die('Remote access disabled in wp-config!');

On line 1987 you have:

wp_die('Sorry, this access token has expired. Please ask the user to renew it.');

On line 1990 you have:

wp_die('Sorry, this is an invalid access token. You must ask the user to grant access.');

    • Niklas
      • The Incredible Code Injector

      I understood that from examining the code, what I don't see is how it was necessary to create a second capability check when add_submenu_page() already contains one.

      You could even return a string from a function that is dependent upon your current user object.

      Pseudo-code:

      add_submenu_page($a, $b, $c, current_user_capability(), …);
      
      function current_user_capability() {
        // if current user is remote support
        return 'remote_capability_string';
        // If current user is regular user
        return 'regular_capabiity_string';
      }

      The reason I ask you check into this is that add_submenu_page() returns and processes more data than necessary and WordPress doesn't exit properly when returning from the callback function.

  • Aaron
    • CTO

    The capability check is only for handling menu registration and display. You still have to protect the actual callback, there is no associated capability check in wp-admin/admin.php.

    This is especially a problem when you don't pass a callback function and it includes a file from the plugin, or often when you are using the same function callback to put a page in different contexts for multisite vs single site. It's very possible for a user to access a page outside their capability if they know the associated url. We've had to patch this security hole in a number of our plugins. There is a very good reason this is noted in the codex for both add_menu_page() and add_submenu_page()!

    • Niklas
      • The Incredible Code Injector

      The dashboard is a nice module and it provides a good glance on the dashboard, but considering the security implications of not using the regular WP access checks and sending more data to the user than necessary and processing more content than necessary it is quite obvious that a fix is needed.

      Will you accept patches if I provide complete bug fixes if you (I assume you were the maintainer) don't want to take the time to fix the bugs or maintain the plugin yourself? I will be more than happy to contribute to avoid that the plugin falls into decay.

      Let's not argue, let's find a solution. Ok?

  • Aaron
    • CTO

    Of course we accept any patches or feedback, but inclusion will be based on our code guidlines and business need. Dashboard is our most important/actively maintained plugin which is why I (CTO/Lead dev) am responsible for it.

    the security implications of not using the regular WP access checks and sending more data to the user than necessary and processing more content than necessary it is quite obvious that a fix is needed.

    No idea what you mean here.

    We are using apt capability checks in the correct places. It is very important to check capability in admin page output code, not just menu registration because admin page output code can often be triggered outside the menu api context, or the menu api context you expect. That's why we do it and why it's in the codex.

  • Niklas
    • The Incredible Code Injector

    What are the code guidelines? I searched but most of what came up was regular blog posts of yours.

    No idea what you mean here.

    The (code logic/design) bug as it stands today introduces at least three issues:

    1. Unnecessary complexity. KIS is our friend as you know. Nothing major.
    2. Since you delay your home-brewed additional access check further WP actions are triggered. This is what I mean by unnecessary code execution further into the menu and admin system, thereby exposing more code paths. This is inherent from #1 where you now need to make additional checks that would not otherwise be necessary.
    3. By sidestepping WP's default capability checks you have created duplicate code paths into the callback function.

    I checked your quoted Codex sentence, and it was added here and refers to pre-3.2 behaviour of WP's menu system. If you really need duplicate paths to reach your content function, then that's fine, but from the few comments in the code and what I can gather from studying it it does not seem so. It seems like we always want to call the content of the plugin in question from the admin interface, which means we can actually solve the access check using WP's recommended method.

    It's not my plugin, but I would really like to keep it installed! Thanks for your time!

    PS. Why does this issue keeps getting marked as Resolved when it is not?

Thank NAME, for their help.

Let NAME know exactly why they deserved these points.

Gift a custom amount of points.