After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 670489 - Incorrect check in check_origin_and_protocol?
Incorrect check in check_origin_and_protocol?
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-02-20 23:18 UTC by Jeff Epler
Modified: 2012-02-21 16:03 UTC
See Also:
GNOME target: ---
GNOME version: 3.1/3.2


Attachments
browser-plugin: Correct check for checking the hostname/protocol (1.01 KB, patch)
2012-02-20 23:24 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jeff Epler 2012-02-20 23:18:27 UTC
It looks as though the check on line 107 should be for 'location', not 'document' (again).  In principle, this lets the code proceed to line 110, calling get_string_property with an inappropriate argument.

 90   error = funcs.getvalue (instance, NPNVWindowNPObject, &window);
 91   if (error != NPERR_NO_ERROR)
 92     goto out;
 93  
 94   if (!funcs.getproperty (instance, window,
 95                           funcs.getstringidentifier ("document"),
 96                           &document))
 97     goto out;
 98  
 99   if (!NPVARIANT_IS_OBJECT (document))
100     goto out;
101  
102   if (!funcs.getproperty (instance, NPVARIANT_TO_OBJECT (document),
103                           funcs.getstringidentifier ("location"),
104                           &location))
105     goto out;
106  
107   if (!NPVARIANT_IS_OBJECT (document))
108     goto out;
109 
110   hostname = get_string_property (instance,
111                                   NPVARIANT_TO_OBJECT (location),
112                                   "hostname");

This is gnome-shell 3.2.2.1 on debian testing, though I quickly verified that this block of code is the same in the master branch of gnome git: http://git.gnome.org/browse/gnome-shell/tree/browser-plugin/browser-plugin.c#n107
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-02-20 23:24:03 UTC
Created attachment 208069 [details] [review]
browser-plugin: Correct check for checking the hostname/protocol

While it's extremely unlikely that document.location would not be an
object in the browser setting, this check is incorrect and we could
possibly crash an NPAPI host if this is the case.


Nice catch.
Comment 2 Owen Taylor 2012-02-21 01:12:00 UTC
Review of attachment 208069 [details] [review]:

Looks good

(when I reviewed this code originally, I'm sure I checked that document.location isn't modifiable from JS, since that would allow a page to defeat the checks)
Comment 3 Jasper St. Pierre (not reading bugmail) 2012-02-21 09:06:38 UTC
Attachment 208069 [details] pushed as 9400d8f - browser-plugin: Correct check for checking the hostname/protocol
Comment 4 Jeff Epler 2012-02-21 16:03:07 UTC
Thank you for your attention to this matter.