[PATCH 1/6] reg: Sanitize key paths in main

Stefan Dösinger stefandoesinger at gmail.com
Tue Oct 28 15:51:10 CDT 2014


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2014-10-28 12:21, schrieb Jonathan Vollebregt:
>> Inconsistent curly bracket placement. The other patches have more
>>  cases of that.
> 
> Should the arrays also have the opening brace on it's own line? A 
> quick grep shows both styles for arrays in wine.
The one line arrays (e.g. short_hklm) no, the two-line or longer ones yes.

> The tests show that having any backslash at the start results in an
>  error. That particular if just picks which error message to show.
Yes, but you could just check for two backslashes and proceed normally
otherwise. You'll then write the same error message because you cannot
find "\\HKEY_CURRENT_USER" in your root key table.

> reg/tests/reg.c:109 shows only the winXP version accepts multiple 
> trailing backslashes.
> 
> The reason for stripping a single trailing backslash was for output
>  consistency with query (As yet unimplemented) Specifically whether
>  query outputs "HKEY_CURRENT_USER\\key" or
> "HKEY_CURRENT_USER\\key\\" as the key name.
It'd be better to add this functionality together with query support
then. Maybe keep it inside query only.

Is there a advapi32 function go get a string name for an open HKEY
handle? You could use that to normalize printed paths. That could also
take care of e.g. upper and lower case letters.

> Because the input is the full path we have to stop comparing after
>  the root key or it will always return 0 unless it's passed a root
>  key.

> I think the "path" parameter name makes this more obvious, but I 
> could throw a comment in if that helps.
Nah, it should be possible to make this function readable without a
comment. How about the names "input_path" and "root_key_name"?

> Actually this solves 2 separate problems. RegOpenKey takes the path
>  after the root key but the path provided includes the root key, so
>  strchrW skips the root key in the path in advance of passing it to
>  RegOpenKey.
Yes, you need the strchrW call.

> On the other hand, if someone inputs only a root key IE "HKCU" it 
> will return NULL. We already have the key from path_get_rootkey, so
>  we just return it here.
I don't think you need the NULL check with the early return, and I
don't think you need to strip the trailing \\ in sanitize_path.
Removing those (not accounting for reg query for now) will remove two
code branches that need testing and make the code more difficult to read.

That's assuming RegOpenKey behaves as intended in those cases:

Msdn says RegOpenKey(key_in, NULL, &key) returns key == key_in, so no
need for the early return in the case that strchrW returns NULL. If
the behavior is the same in RegOpenKey(key_in, "\\", &key) there's no
need to strip trailing backslashes.

Maybe RegOpenKey(NULL, xxx, &key) returns an error that you can
propagate to main() and translate into an error message there. In that
case you don't have to write an error message from a helper function.

The RegOpenKey behavior may need additional tests in advapi32.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJUUAG+AAoJEN0/YqbEcdMwD+QP/3sHl1+Xs1pMBDjThviSv5mg
ohPDgCBIKFLlAT6bP+jF+ds/B6coVtLFCdr8ocUXZrFN9v6nN2k8r5z87QdN6xdb
oVfjj9sabZUvDd/PSrtzXAHz7Vfk4Db8kN6YuuZJMrNAhcYb/63cn25r5Zc3YvtV
UV8rJtx79p/xzT4vRXo3ESWOrUcjMknylSsEzypLIn3ihvZYDWODhGPg2Qwk7V9n
lyru5I/rpak1W33a+sO7/5Z4hKHTseyZjdChlXVFmip8JPLG8mImweMNSrxgdqF/
/hh7DD+krbl1+IJPJ7dxrBklocjeMtpaDZH2a1NHDzCDBCV39MZgmj0h3WQkGH39
rZraihHKEbXDnZP4ZVZVC8CUZskDfQt8dqZ5eW0IUqil0upJPFvhm8Dl9QpYu2mr
lw1CV5+6ZTmS/S3sy4Q6DQ6v5t1HjNvMBbUPKVWWt6HW7mgdEgaZmdFOdHqwKgCS
hVzO0u7qjPp7hPPLcWS/+ZgeImN4enhBzuB9GZ1NLKe+fix9FkLwU+jjeWWrTcJU
g8Q+Oa3r8Xf6drW894V4uCkPkHFH+VjPllpQpiSjnIammb5QXBW/cCZHdT0RF5eG
7qv5YcBc3jlerllzVrcj8uHR+jmhtm7aDIKOh87e6M+RuPGHG2C5rLTEdMEf5HW0
OuNrs5CvHNMmv7Mn+3UH
=Mbi0
-----END PGP SIGNATURE-----



More information about the wine-devel mailing list