obsolete RegOpenKey API used

RESOLVED WORKSFORME

Status

RESOLVED WORKSFORME
15 years ago
9 years ago

People

(Reporter: brant, Assigned: Bienvenu)

Tracking

Trunk
x86
Windows XP
Bug Flags:
blocking1.8a1 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b) Gecko/20030808 Mozilla Firebird/0.6.1+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b) Gecko/20030808 Mozilla Firebird/0.6.1+

The RegOpenKey API is being used.  It is obsolete and RegOpenKeyEx should be
used in its place.

Reproducible: Always

Steps to Reproduce:
1. Search for RegOpenKey with lxr.
2. One of the files is PalmSyncInstall.cpp
Actual Results:  
This file should not use RegOpenKey.

Expected Results:  
It should us RegOpenKeyEx instead.

I made a best guess at what access permission is necessary but most limited in
my patch.
(Reporter)

Comment 1

15 years ago
Created attachment 129917 [details] [diff] [review]
Patch that changes RegOpenKey to RegOpenKeyEx
(Reporter)

Comment 2

15 years ago
Comment on attachment 129917 [details] [diff] [review]
Patch that changes RegOpenKey to RegOpenKeyEx

Since I can't compile Mozilla (no MSVC and Mingw that hasn't worked yet),
bsmedberg said for me to request a review from him to test the patch before
actual reviews requested.
Attachment #129917 - Flags: review?(bsmedberg)

Comment 3

15 years ago
Comment on attachment 129917 [details] [diff] [review]
Patch that changes RegOpenKey to RegOpenKeyEx

looks good to me... who owns this code so you can get owner-approval?
Attachment #129917 - Flags: review?(bsmedberg) → review+

Comment 4

15 years ago
Comment on attachment 129917 [details] [diff] [review]
Patch that changes RegOpenKey to RegOpenKeyEx

Noticed this bug when looking at bug 237754 for Firefox. This patch is for Palm
Sync. I'll put together one for winhooks. It's been stagnant for a while so
would be good to get this checked in.
Attachment #129917 - Flags: superreview?(scc)

Comment 5

15 years ago
Created attachment 146228 [details] [diff] [review]
Patch for winhooks and MAPI RegOpenKey -> RegOpenKeyEx

Supplemental patch to the other patch, this one covers winhooks and mapi.

I think that's all the instances in Seamonkey where RegOpenKey is incorrectly
called

Comment 6

15 years ago
Not sure of the correct component but it definitely has nothing to do with the
installer so moving to XP Apps.

If anyone has any ideas who could review my new patch then please add the
appropriate flag.

Should be easy enough to verify and possibly make it into 1.8a.

MSDN on RegOpenKey:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/sysinfo/base/regopenkey.asp
Assignee: ssu0262 → jag
Component: Installer: XPInstall Engine → XP Apps
Flags: blocking1.8a?
QA Contact: agracebush → pawyskoczka
(Reporter)

Comment 7

15 years ago
Comment on attachment 146228 [details] [diff] [review]
Patch for winhooks and MAPI RegOpenKey -> RegOpenKeyEx

For the second portion of your patch (nsWindowsHookUtil.cpp), you are giving
KEY_ALL_ACCESS permissions to the operation, however, I don't think it needs
that much permission.  I think those permissions are are bit masks, so or'ing
them for the specific tasks needed will be better than all access.  I might be
wrong on this though, I'm not a C/C++ programmer yet.

Updated

15 years ago
Flags: blocking1.8a? → blocking1.8a-

Comment 8

15 years ago
KEY_READ = KEY_QUERY_VALUE | KEY_ENUMERATE_SUB_KEYS | KEY_NOTIFY

KEY_WRITE = KEY_SET_VALUE | KEY_CREATE_SUB_KEY

KEY_ALL_ACCESS = KEY_QUERY_VALUE | KEY_ENUMERATE_SUB_KEYS | KEY_NOTIFY |
                 KEY_CREATE_SUB_KEY | KEY_CREATE_LINK | KEY_SET_VALUE

Therefore KEY_ALL_ACCESS = KEY_READ | KEY_WRITE | KEY_CREATE_LINK.  That's not
that big of a deal, but maybe KEY_ALL_ACCESS could be changed to just KEY_READ |
KEY_WRITE?

Comment 9

15 years ago
The real test to do here is to test the code with a non-privileged account (no
administrator rights). KEY_ALL_ACCESS might only work with administrator
account. Any other flags that do the job for a non-privileged user is OK.
Product: Core → Mozilla Application Suite
(Assignee)

Comment 10

13 years ago
I'll look at driving this home.
Assignee: jag → bienvenu
(Assignee)

Comment 11

9 years ago
I don't see any uses of RegOpenKey in comm-central.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → WORKSFORME
Comment on attachment 129917 [details] [diff] [review]
Patch that changes RegOpenKey to RegOpenKeyEx

Per comment 11, RegOpenKey is no longer present in the source. Therefore clearing obsolete review request.
Attachment #129917 - Flags: superreview?(scc)
You need to log in before you can comment on or make changes to this bug.