Security issue with using ssl and gravatar

Issue #233 resolved
C F created an issue

Hi,

When zurmo is served over ssl and gravatar is used for avatar images, the browser emits security warnings.

It would be great if one could change the gravatar URL in a file or better yet if the app detected an ssl connection and used the https://www.gravatar.com url.

-Thanks!

Comments (15)

  1. C F reporter

    Sure. I'll also include the content below.

    --- /zurmo/app/protected/modules/users/models/User.php.old  Fri Jan 10 01:41:00 2014
    +++ /zurmo/app/protected/modules/users/models/User.php  Mon Feb 10 20:25:27 2014
    @@ -415,23 +415,37 @@
                     {
                         $avatar = unserialize($this->serializedAvatarData);
                     }
    +                // TODO: a config option should be added so SSL can be forced on or off
    +                if ( isset($_SERVER['HTTPS']) && ($_SERVER['HTTPS'] != 'off' || !$_SERVER['HTTPS']))
    +                {
    +                    $scheme = 'https';
    +                }
    +                else
    +                {
    +                    $scheme = 'http';
    +                }
    +                $baseGravatarUrl = $scheme . '://www.gravatar.com/avatar/%s?s=' . $size . '&r=g';
    +                $gravatarUrlFormat        = $baseGravatarUrl . '&d=identicon';
    +                $gravatarDefaultUrlFormat = $baseGravatarUrl . '&d=mm';
                     if (isset($avatar['avatarType']) && $avatar['avatarType'] == User::AVATAR_TYPE_DEFAULT)
                     {
    -                    $avatarUrl = "http://www.gravatar.com/avatar/?s={$size}&r=g&d=mm"; // Not Coding Standard
    +                    $avatarUrl = sprintf($gravatarDefaultUrlFormat, '');
                     }
                     elseif (isset($avatar['avatarType']) && $avatar['avatarType'] == User::AVATAR_TYPE_PRIMARY_EMAIL)
                     {
                         $email      = $this->primaryEmail->emailAddress;
    -                    $avatarUrl   = "http://www.gravatar.com/avatar/" . md5(strtolower(trim($email))) . "?s={$size}&d=identicon&r=g"; // Not Coding Standard
    +                    $emailHash  = md5(strtolower(trim($email)));
    +                    $avatarUrl  = sprintf($gravatarUrlFormat, $emailHash);
                     }
                     elseif (isset($avatar['avatarType']) && $avatar['avatarType'] == User::AVATAR_TYPE_CUSTOM_EMAIL)
                     {
                         $email      = $avatar['customAvatarEmailAddress'];
    -                    $avatarUrl   = "http://www.gravatar.com/avatar/" . md5(strtolower(trim($email))) . "?s={$size}&d=identicon&r=g"; // Not Coding Standard
    +                    $emailHash  = md5(strtolower(trim($email)));
    +                    $avatarUrl  = sprintf($gravatarUrlFormat, $emailHash);
                     }
                     else
                     {
    -                    $avatarUrl = "http://www.gravatar.com/avatar/?s={$size}&r=g&d=mm"; // Not Coding Standard
    +                    $avatarUrl = sprintf($gravatarDefaultUrlFormat, '');
                     }
                     //Check connection to gravatar and return offline picture
                     $htmlHeaders = @get_headers($avatarUrl);
    @@ -1036,4 +1050,3 @@
                 return false;
             }
         }
    -?>
    \ No newline at end of file
    
  2. Jason Green

    Can you refactor this to utilize the YII::app() to get the HTTP vs. HTTPS schema. we can get that from the Yii App instead of checking SERVER in this method, which would be better.

  3. C F reporter

    i've changed the check to:

    if (YII::app()->getRequest()->getIsSecureConnection())
    

    I will attach a new patch.

  4. Chris Edwards

    The browser will get the image using $baseGravatarUrl = '//www.gravatar.com/avatar/%s?s=' . $size . '&r=g'; So it will use the same transport as the calling page (https or http).

    The reason the last commit wasn't working was because get_headers() wouldn't understand a url without a transport.

    The get_headers() call is done by the server, not the client and so it does not matter which transport is used. The browser will not show any errors if http is used even on an https site since the http call would be done by the hosting server. So we specify http so that get_headers() has a valid transport.

    I guess that get_headers() is used in case gravatar is offline? If that is not a concern we could probably just get rid of the get_headers() since gravitar will return a generic icon if one is not found for the user and I don't think the response code changes depending on if an icon is found?. I don't know if the performance / benefit is actually worth it by checking if gravatar is online or not? Or perhaps this is there to future proof for an offline zurmo UI?

  5. Log in to comment