2009/1/26 Aurimas Fišeras <[email protected]>: > Saturn's error report: > (INCONSISTENT USE) Possible null dereference of variable data+(count-1). > This variable is checked for Null at lines: registry.c:1051 > > Tested on Windows XP > > Changelog: > advapi32: Fix potential NULL pointer dereference in RegSetValueExA > [with test] (Saturn)
Excellent, this tool has spotted a corner-case that the code doesn't handle correctly. > From ea7773cc046992e327030fb99935afc5b25c1b4b Mon Sep 17 00:00:00 2001 > From: Aurimas Fischer <[email protected]> > Date: Mon, 26 Jan 2009 21:55:05 +0200 > Subject: advapi32: Fix potential NULL pointer dereference in RegSetValueExA > [with test] (Saturn) > > --- > dlls/advapi32/registry.c | 1 + > dlls/advapi32/tests/registry.c | 4 ++++ > 2 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/dlls/advapi32/registry.c b/dlls/advapi32/registry.c > index 52de6c5..88a89db 100644 > --- a/dlls/advapi32/registry.c > +++ b/dlls/advapi32/registry.c > @@ -1055,6 +1055,7 @@ LSTATUS WINAPI RegSetValueExA( HKEY hkey, LPCSTR name, > DWORD reserved, DWORD typ > else if (count && is_string(type)) > { > /* if user forgot to count terminating null, add it (yes NT does > this) */ > + if (!data) return ERROR_NOACCESS; This should be moved before the comment to avoid the comment relating to the wrong line. > if (data[count-1] && !data[count]) count++; > } > > diff --git a/dlls/advapi32/tests/registry.c b/dlls/advapi32/tests/registry.c > index b63b3e2..0e1b673 100644 > --- a/dlls/advapi32/tests/registry.c > +++ b/dlls/advapi32/tests/registry.c > @@ -383,6 +383,10 @@ static void test_set_value(void) > test_hkey_main_Value_A(name2A, string2A, sizeof(string2A)); > test_hkey_main_Value_W(name2W, string2W, sizeof(string2W)); > > + /* test RegSetValueExA with invalid parameters*/ > + ret = RegSetValueExA(hkey_main, name1A, 0, REG_SZ, NULL, 1); > + ok(ret == ERROR_NOACCESS, "got %d (expected ERROR_NOACCESS)\n", ret); From the source of RegSetValueExA, this appears to return ERROR_INVALID_PARAMETER on Win9x, so this test case needs to be run on Win9x and fixed if necessary to take this into account. -- Rob Shearman
