MediaVimeoStreamWrapper should use json API [#2079261]

static function getVideoProperties($video_id) {
    // The .php format returns a serialized array.
    $response = drupal_http_request('http://vimeo.com/api/v2/video/'. $video_id .'.php');
    return unserialize($response->data);
  }

VIMEO, or anyone owning the domain, can now delete arbitrary files from the files directory. Depending on the objects available for autoload, VIMEO, or anyone owning the domain, may do even worse stuff, such as executing code.

Please use the JSON API instead.

Comment	File	Size	Author
#3	media_vimeo-switch_to_json-2079261-3.patch	1.19 KB	FatherShawn

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

FatherShawn

English

New York

CreditAttribution: FatherShawn commented 12 September 2013 at 12:21

As a beta module, this doesn't fall under the security team's purview but it's still not proper to post publicly a security concern... That said, I don't see the concern since drupal_http_request returns a object with the result of the request. The processing happens at vimeo...

If wiser hackers see a concern, I'm happy to post a patch that uses json instead.

Comment #2

Heine CreditAttribution: Heine commented 12 September 2013 at 17:12

This issue can be discussed in public because it involves a module that has no release supported by the security team.

The concern is as follows:$request->data contains a string delivered by "vimeo.com" that is passed directly to unserialize.

If the string contains an "object definition", the call to unserialize will instantiate such an object with the provided values. The objects __wakeup method will be called, if it exists. When the object is destroyed (eg when it goes out of scope), the classes __destruct function will be called, if it exists. The object will contain memberdata (even private) provided by "vimeo.com". The userprovided data can cause the destructor to do rather unexpected things.

See http://heine.familiedeelstra.com/security/unserialize for an example.

Decoding JSON has no such trouble.

Comment #3

FatherShawn

English

New York

CreditAttribution: FatherShawn commented 14 September 2013 at 21:20

Assigned:	Unassigned	» FatherShawn
Status:	Active	» Needs review

File	Size
media_vimeo-switch_to_json-2079261-3.patch	1.19 KB

Here's the code, switched to JSON. Tested in my local environment...

Comment #4

Yaron Tal CreditAttribution: Yaron Tal commented 9 December 2013 at 13:41

Issue summary:	View changes
Status:	Needs review	» Reviewed & tested by the community

Above patch works for me, and also fixes a fatal error when no such video exists on vimeo or when the video is not public (#1535518: Private Video). The image will still not be displayed in that case, but we'll at least not get any fatal error.