wp_get_nav_menu_object() doesn't check when passing to get_term()

[kawauso]

wp_get_nav_menu_object() initially passes the menu string to get_term() to see if it's an ID. However, get_term() expects an integer or object and type-casts any non-objects passed into integers. This results in a menu name i.e. '10-ton elephant' being reduced to '10', which it then uses as a term ID, potentially producing unexpected results.

[kawauso]

Patch sets $menu_obj to false initially and removes the redundant end false check (get_term_by() will return an object or false, if get_term() has passed null then it causes a get_term_by() call). Also adds an is_numeric() check to get_term().

[kawauso]

Steps to reproduce:

1. Add a nav menu and find its term ID
2. Call wp_get_nav_menu_object() with a string beginning with that ID
3. The nav menu will be returned despite the passed string not being a valid menu name

[kawauso]

wp_get_nav_menu_to_edit() incorrectly passed the menu object to is_nav_menu() (it is only meant to take an id, slug or name). Following patch adds check logic from is_nav_menu() in place of the is_nav_menu() call.

wp_get_nav_menu_to_edit() can still return null when the conditions are not met, so the PHPDoc @return should be expanded to reflect this or handling added to convert null values into WP_Error objects.

[kawauso]

s/menu_obj/menu

[cburtbcit]

I can confirm that I have run into this bug as well. The diff provided appears to solve the problem. I don't think it's that unlikely that someone would name a page with an integer as the first character or two in the title so in my opinion it's important that this patch be included in the next release of WordPress. I see that the version number for the bug is listed as 3.1.4 but I'm experiencing this in 3.2.1 as well. Thank you!

[kawauso]

Replying to cburtbcit:
The Version refers to the earliest version affected by the bug.

[duck_]

Replying to kawauso:

wp_get_nav_menu_to_edit() incorrectly passed the menu object to is_nav_menu() (it is only meant to take an id, slug or name). Following patch adds check logic from is_nav_menu() in place of the is_nav_menu() call.

Although passing an object was undocumented should we be careful about backwards compatibility here? As you say even core passed an object to is_nav_menu() / wp_get_nav_menu_object().

[cburtbcit]

I've recently had to apply the patch provided by kawauso once again. It still works and does not seem to produce any unexpected behavior. What's keeping this from being included in the next update? Thanks!

[xandanet]

Ran across this again in 4.0.1. Can anyone verify?

I worked around the unexpected behaviour by skipping straight to:

get_term_by( 'name', $menu_title, 'nav_menu' );

instead, from my plugin code.