• meh1936

    (@meh1936)


    It seems that the hardening has broken password reset links. Here’s a redacted link generated from a reset:
    https://(domain.com)/your-account/?a=set_password_from_key\&\#\0\3\8\;key=(redacted)\&\#\0\3\8\;login=(redacted)

    Escaping the code because this forum is just converting it to an ampersand.

    The page generates:
    Invalid user.
    Request a new reset key.

    It works as expected if the user modifies the URL to just the ampersands instead of the \&\#\0\3\8\;

    • This topic was modified 1 month ago by meh1936.
    • This topic was modified 1 month ago by meh1936.
Viewing 12 replies - 1 through 12 (of 12 total)
  • airdrummer

    (@airdrummer)

    seems to have broken passwd reset emails on my site-(none sent)-;

    rolled back to v3.4.9whatever, now i’m getting invalid keys-\

    Plugin Author Chad Butler

    (@cbutlerjr)

    Thanks for the update. I’ll look into it and post a patch for this as shortly. Then I’ll work it into the plugin.

    airdrummer

    (@airdrummer)

    patch fixed it thanx!

    Plugin Author Chad Butler

    (@cbutlerjr)

    So… after testing, I have not been able to reproduce the output you indicated. In fact, I have not been able to reproduce an invalid output result at all.

    Scenarios tested:

    • Default (plugin inserts link automatically)
    • [reset_link] shortcode
    • HTML formatted email

    Each of those tests yielded a valid reset link.

    I believe the only difference between 3.5.0 and 3.4.9.7 is it was previously using esc_url_raw() to escape the link, now that is esc_url().

    Note that both of those (and a number of other functions used in the process) are core WP functions. That means there can be filter hooks that are triggered that result in different outcomes based on your install. Similar with other situation-specific things like PHP version, web server, etc. So before I go publishing patches to chase something uncertain, I’ll ask you to validate and test with the following:

    In 3.5.0, go to includes/class-wp-members-pwd-reset.php and at line 213, change esc_url to esc_url_raw and retest. Then let me know the outcome.

    • This reply was modified 1 month ago by Chad Butler. Reason: specific as to which version the test is for
    airdrummer

    (@airdrummer)

    sorry i’m juggling safari & firefox & can’t keep anything straight right now

    • This reply was modified 1 month ago by airdrummer.
    • This reply was modified 1 month ago by airdrummer.
    Plugin Author Chad Butler

    (@cbutlerjr)

    Unfortunately, that doesn’t help – that just shows what has already been noted by the OP. I already believe the output as stated – but as I said in my response, my tests do not reproduce the issue, which makes it impossible to pin down specifically to the plugin. That’s not saying it’s not a change in the plugin that is needed, but that without being able to reproduce the issue you’re having, I have zero clue on the exact cause – only guesses – and there are several WP core functions and filters in there that mean it could be something in another plugin, custom code snippet, theme, or any number of things. Or, it might be as simple as using esc_url_raw() instead of esc_url()

    Make the change noted (last line of my previous response) and compare the outcome. That’s the info that I need to make any forward progress on this.

    Also, all that being said, it doesn’t make a whole lot of sense why it’s only between the second/third query args and not between the first/second. It makes me wonder what’s coming back from get_password_reset_key() (the WP function that generates the key) and what’s in $user->user_login (also a WP value). So an additional test would be to add the following:

    Immediately before line 196:

    $query_args = array_map( 'rawurlencode', $query_args );

    Add the following immediately before that (not after):

    $query_args = array_map( 'trim', $query_args );

    That’s a separate test. It needs to be separate from the first one (esc_url vs esc_url_raw)

    airdrummer

    (@airdrummer)

    just generated https://seventyone4fun4ever.letartliveon.com/home/?a=set_password_from_key#038;key=4PKY1yZCOxmZ92jNqhFG&%23038;login=xxxx

    and get Invalid user…i’ll try ur debug suggestion

    • This reply was modified 1 month ago by airdrummer.
    airdrummer

    (@airdrummer)

    changed esc_url to esc_url_raw & pw reset link works now

    • This reply was modified 1 month ago by airdrummer.
    airdrummer

    (@airdrummer)

    i see what the problem is on my end: i’m catching the emails on my phone,

    then sending the url via msg to my laptop, but every new url window seems to munge it:

    https://asapackermansion.letartliveon.com/home/login/?a=set_password_from_key&key=jisfQZsyp02xDovq9Fpj&%23038;login=lehigh71classcorrespondent

    note the 1st & gets fixed, and the 2nd gets munged to &%23038; but the extraneous #038;s are gone after replacing esc_url() with esc_url_raw()

    airdrummer

    (@airdrummer)

    cut&pasted from the url in the email inthe image above:

    https://asapackermansion.letartliveon.com/home/login/?a=set_password_from_key&key=jisfQZsyp02xDovq9Epj&#038/login=lehigh71classcorrespondent

    note 1st “&# 038;” got fixed by this input box (note space to prevent munging)

    • This reply was modified 1 month ago by airdrummer. Reason: unmunge
    • This reply was modified 1 month ago by airdrummer. Reason: (note space to prevent munging)
    Thread Starter meh1936

    (@meh1936)

    In 3.5.0, go to includes/class-wp-members-pwd-reset.php and at line 213, change?esc_url?to?esc_url_raw?and retest. Then let me know the outcome.

    I can confirm that this change works on my install.

    Plugin Author Chad Butler

    (@cbutlerjr)

    Thanks for the update!

Viewing 12 replies - 1 through 12 (of 12 total)
  • You must be logged in to reply to this topic.