Jump to:
| Project: | Services |
| Version: | 6.x-0.15 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (works as designed) |
Issue Summary
Hi,
First of all thanks for this great module :)
Maybe there is something i miss here but i don't understand how passing the session id actually helps the security of the service.
I looked at the code and did some testing using flex and amfphp - it seems that i can pass whatever session-id that i want - meaning i only get an error if i pass an empty session id. there is no comparison between the session id that is passed and the actual session id of the current global user.
if that is really the case a recommend adding to the services_method_call this section
if ($method['#auth'] and variable_get('services_use_sessid', TRUE)) {
$sessid = array_shift($args);
global $user;
if (empty($sessid) || ($user->uid != 0 && $user->sid != $sessid)) {
return services_error(t('Invalid sessid.'));
}
$session_backup = services_session_load($sessid);
}instead of this
if ($method['#auth'] and variable_get('services_use_sessid', TRUE)) {
$sessid = array_shift($args);
if (empty($sessid)) {
return services_error(t('Invalid sessid.'));
}
$session_backup = services_session_load($sessid);
}by doing so every authenticated user call a comparison between the session ids take place.
for unauthenticated user i believe that the best way is not to allow sensitive services call by using the user access argument of the service.
tell me what you think - ill be happy to create a patch if my solution seems good for everyone else except me ...
Comments
#1
I agree that there should be some verification of identity. Subscribe
#2
This solution doesn't make sense to me. When accessing a remote server from a client, the server's $user makes absolutely no sense, since there is no persistent user. This is why, when using user sessions, you have to pass a session id with every request. Your solution may work with clients that support cookies, but not all clients do (for instance Drupal's own XMLRPC client does not.) So I don't really get what you hope to acheive by adding the global $user line there.
In terms of the session IDs, while it is true you can pass any session id without error, that session is evaluated with the anonymous user's permissions. Passing in an existing session id is the only way to access any data that isn't already available to anyone.
I don't think there's a problem here.