Stealing cookies: unhelpful check of $yourls_user_passwords #2122

Open
tipichris opened this Issue Jul 19, 2016 · 0 comments

Projects

None yet

1 participant

@tipichris

Technical details

  • YOURLS version: 1.7.1
  • PHP version: 5.5.9

Reproducible Bug Summary

This issue affects the ability to write plugins providing alternative auth mechanisms. The steps to reproduce given below therefore involve installing a plugin. However, I consider this to be a bug in the core code rather than the plugin

  1. Install and configure yourls-ldap-plugin
  2. Do not configure LDAPAUTH_ADD_NEW option, which adds LDAP users to the list of users in users/config.php
  3. Login using an account stored in LDAP
  4. The result is that YOURLS dies with the single line "Stealing cookies?"

This is the result of a check in yourls_store_cookie() for the presence of an appropriate entry in the $yourls_user_passwords array, which is used to configure users and passwords in users/config.php. A comment suggests that the die is called because "this should never happen", but if a plugin is handling authentication, then there is no reason for it not to happen.

Plugins using auth mechanisms such as Shiboleth or OAuth will never have knowledge of the user passwords. Other plugins, such as LDAP, will only have knowledge of it when actually handling the login form, not at any other time, unless they undertake an expensive call to the backend. Consequently they can not set anything meaningful as a value in $yourls_user_passwords.

What this check appears to be doing really is ensuring that the value of $user passed into yourls_store_cookie() is present in the list of users configured in users/config.php (note that although the code sets the value of the locally scoped $pass here, it never does anything with that). If auth is handled by a plugin there is no reason why the value of $user should be configured in users/config.php. The place to check if a user is valid is in yourls_is_valid_user(). The check should already have been done, filtered by the plugin, by the time yourls_store_cookie() is called.

This check should be removed from yourls_store_cookie()

@tipichris tipichris referenced this issue in k3a/yourls-ldap-plugin Jul 19, 2016
Open

"Stealing cookies?" problem #8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment