Skip to content

W
wine-winehq
Commit 9032eeec authored by Esme Povirk's avatar Esme Povirk Committed by Alexandre Julliard
Browse files
Options

sechost: Reject string SIDs with too many characters.

Signed-off-by: 's avatarEsme Povirk <esme@codeweavers.com> Signed-off-by: 's avatarAlexandre Julliard <julliard@winehq.org>
parent ce94c78b
Hide whitespace changes
Inline Side-by-side
Showing with 51 additions and 24 deletions
dlls/advapi32/tests/security.c
View file @ 9032eeec
...@@ -338,6 +338,20 @@ static void test_ConvertStringSidToSid(void) ...@@ -338,6 +338,20 @@ static void test_ConvertStringSidToSid(void)
"expected GetLastError() is ERROR_INVALID_SID, got %d\n", "expected GetLastError() is ERROR_INVALID_SID, got %d\n",
GetLastError() ); GetLastError() );
r = ConvertStringSidToSidA( "WDandmorecharacters", &psid );
ok( !r,
"expected failure with too many characters\n" );
ok( GetLastError() == ERROR_INVALID_SID,
"expected GetLastError() is ERROR_INVALID_SID, got %d\n",
GetLastError() );
r = ConvertStringSidToSidA( "WD)", &psid );
ok( !r,
"expected failure with too many characters\n" );
ok( GetLastError() == ERROR_INVALID_SID,
"expected GetLastError() is ERROR_INVALID_SID, got %d\n",
GetLastError() );
ok(ConvertStringSidToSidA("S-1-5-21-93476-23408-4576", &psid), "ConvertStringSidToSidA failed\n"); ok(ConvertStringSidToSidA("S-1-5-21-93476-23408-4576", &psid), "ConvertStringSidToSidA failed\n");
pisid = psid; pisid = psid;
ok(pisid->SubAuthorityCount == 4, "Invalid sub authority count - expected 4, got %d\n", pisid->SubAuthorityCount); ok(pisid->SubAuthorityCount == 4, "Invalid sub authority count - expected 4, got %d\n", pisid->SubAuthorityCount);
...@@ -4237,7 +4251,8 @@ static void test_ConvertStringSecurityDescriptor(void) ...@@ -4237,7 +4251,8 @@ static void test_ConvertStringSecurityDescriptor(void)
{ "", SDDL_REVISION_1, TRUE }, { "", SDDL_REVISION_1, TRUE },
/* test ACE string SID */ /* test ACE string SID */
{ "D:(D;;GA;;;S-1-0-0)", SDDL_REVISION_1, TRUE }, { "D:(D;;GA;;;S-1-0-0)", SDDL_REVISION_1, TRUE },
{ "D:(D;;GA;;;Nonexistent account)", SDDL_REVISION_1, FALSE, ERROR_INVALID_ACL, ERROR_INVALID_SID } /* W2K */ { "D:(D;;GA;;;WDANDSUCH)", SDDL_REVISION_1, FALSE, ERROR_INVALID_ACL },
{ "D:(D;;GA;;;Nonexistent account)", SDDL_REVISION_1, FALSE, ERROR_INVALID_ACL, ERROR_INVALID_SID }, /* W2K */
}; };
for (i = 0; i < ARRAY_SIZE(cssd); i++) for (i = 0; i < ARRAY_SIZE(cssd); i++)
......
dlls/sechost/security.c
View file @ 9032eeec
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
*/ */
#include <assert.h>
#include <stdarg.h> #include <stdarg.h>
#include "windef.h" #include "windef.h"
#include "winbase.h" #include "winbase.h"
...@@ -593,18 +594,22 @@ static BOOL get_computer_sid( PSID sid ) ...@@ -593,18 +594,22 @@ static BOOL get_computer_sid( PSID sid )
return TRUE; return TRUE;
} }
static DWORD get_sid_size( const WCHAR *string ) static DWORD get_sid_size( const WCHAR *string, const WCHAR **end )
{ {
if (string[0] == 'S' && string[1] == '-') /* S-R-I(-S)+ */ if (string[0] == 'S' && string[1] == '-') /* S-R-I(-S)+ */
{ {
int token_count = 0; int token_count = 0;
while (*string) string++;
while (*string == '-' || iswdigit(*string))
{ {
if (*string == '-') if (*string == '-')
token_count++; token_count++;
string++; string++;
} }
if (end)
*end = string;
if (token_count >= 3) if (token_count >= 3)
return GetSidLengthRequired( token_count - 2 ); return GetSidLengthRequired( token_count - 2 );
} }
...@@ -612,6 +617,9 @@ static DWORD get_sid_size( const WCHAR *string ) ...@@ -612,6 +617,9 @@ static DWORD get_sid_size( const WCHAR *string )
{ {
unsigned int i; unsigned int i;
if (end)
*end = string + 2;
for (i = 0; i < ARRAY_SIZE(well_known_sids); i++) for (i = 0; i < ARRAY_SIZE(well_known_sids); i++)
{ {
if (!wcsncmp( well_known_sids[i].str, string, 2 )) if (!wcsncmp( well_known_sids[i].str, string, 2 ))
...@@ -632,12 +640,12 @@ static DWORD get_sid_size( const WCHAR *string ) ...@@ -632,12 +640,12 @@ static DWORD get_sid_size( const WCHAR *string )
return GetSidLengthRequired( 0 ); return GetSidLengthRequired( 0 );
} }
static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size ) static BOOL parse_sid( const WCHAR *string, const WCHAR **end, SID *pisid, DWORD *size )
{ {
while (*string == ' ') while (*string == ' ')
string++; string++;
*size = get_sid_size( string ); *size = get_sid_size( string, end );
if (!pisid) /* Simply compute the size */ if (!pisid) /* Simply compute the size */
return TRUE; return TRUE;
...@@ -647,7 +655,7 @@ static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size ) ...@@ -647,7 +655,7 @@ static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size )
DWORD csubauth = ((*size - GetSidLengthRequired(0)) / sizeof(DWORD)); DWORD csubauth = ((*size - GetSidLengthRequired(0)) / sizeof(DWORD));
string += 2; /* Advance to Revision */ string += 2; /* Advance to Revision */
pisid->Revision = wcstoul( string, NULL, 10 ); pisid->Revision = wcstoul( string, (WCHAR**)&string, 10 );
if (pisid->Revision != SDDL_REVISION) if (pisid->Revision != SDDL_REVISION)
{ {
...@@ -665,8 +673,6 @@ static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size ) ...@@ -665,8 +673,6 @@ static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size )
pisid->SubAuthorityCount = csubauth; pisid->SubAuthorityCount = csubauth;
/* Advance to identifier authority */ /* Advance to identifier authority */
while (*string && *string != '-')
string++;
if (*string == '-') if (*string == '-')
string++; string++;
...@@ -675,24 +681,20 @@ static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size ) ...@@ -675,24 +681,20 @@ static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size )
*/ */
pisid->IdentifierAuthority.Value[0] = 0; pisid->IdentifierAuthority.Value[0] = 0;
pisid->IdentifierAuthority.Value[1] = 0; pisid->IdentifierAuthority.Value[1] = 0;
identAuth = wcstoul( string, NULL, 10 ); identAuth = wcstoul( string, (WCHAR**)&string, 10 );
pisid->IdentifierAuthority.Value[5] = identAuth & 0xff; pisid->IdentifierAuthority.Value[5] = identAuth & 0xff;
pisid->IdentifierAuthority.Value[4] = (identAuth & 0xff00) >> 8; pisid->IdentifierAuthority.Value[4] = (identAuth & 0xff00) >> 8;
pisid->IdentifierAuthority.Value[3] = (identAuth & 0xff0000) >> 16; pisid->IdentifierAuthority.Value[3] = (identAuth & 0xff0000) >> 16;
pisid->IdentifierAuthority.Value[2] = (identAuth & 0xff000000) >> 24; pisid->IdentifierAuthority.Value[2] = (identAuth & 0xff000000) >> 24;
/* Advance to first sub authority */ /* Advance to first sub authority */
while (*string && *string != '-')
string++;
if (*string == '-') if (*string == '-')
string++; string++;
while (*string) while (iswdigit(*string) || *string == '-')
{ {
pisid->SubAuthority[i++] = wcstoul( string, NULL, 10 ); pisid->SubAuthority[i++] = wcstoul( string, (WCHAR**)&string, 10 );
while (*string && *string != '-')
string++;
if (*string == '-') if (*string == '-')
string++; string++;
} }
...@@ -703,6 +705,9 @@ static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size ) ...@@ -703,6 +705,9 @@ static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size )
return FALSE; return FALSE;
} }
if (end)
assert(*end == string);
return TRUE; return TRUE;
} }
else /* String constant format - Only available in winxp and above */ else /* String constant format - Only available in winxp and above */
...@@ -746,6 +751,7 @@ static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size ) ...@@ -746,6 +751,7 @@ static BOOL parse_sid( const WCHAR *string, SID *pisid, DWORD *size )
BOOL WINAPI DECLSPEC_HOTPATCH ConvertStringSidToSidW( const WCHAR *string, PSID *sid ) BOOL WINAPI DECLSPEC_HOTPATCH ConvertStringSidToSidW( const WCHAR *string, PSID *sid )
{ {
DWORD size; DWORD size;
const WCHAR *string_end;
TRACE("%s, %p\n", debugstr_w(string), sid); TRACE("%s, %p\n", debugstr_w(string), sid);
...@@ -761,12 +767,18 @@ BOOL WINAPI DECLSPEC_HOTPATCH ConvertStringSidToSidW( const WCHAR *string, PSID ...@@ -761,12 +767,18 @@ BOOL WINAPI DECLSPEC_HOTPATCH ConvertStringSidToSidW( const WCHAR *string, PSID
return FALSE; return FALSE;
} }
if (!parse_sid( string, NULL, &size )) if (!parse_sid( string, &string_end, NULL, &size ))
return FALSE; return FALSE;
if (*string_end)
{
SetLastError(ERROR_INVALID_SID);
return FALSE;
}
*sid = LocalAlloc( 0, size ); *sid = LocalAlloc( 0, size );
if (!parse_sid( string, *sid, &size )) if (!parse_sid( string, NULL, *sid, &size ))
{ {
LocalFree( *sid ); LocalFree( *sid );
return FALSE; return FALSE;
...@@ -996,11 +1008,11 @@ static BOOL parse_acl( const WCHAR *string, DWORD *flags, ACL *acl, DWORD *ret_s ...@@ -996,11 +1008,11 @@ static BOOL parse_acl( const WCHAR *string, DWORD *flags, ACL *acl, DWORD *ret_s
string++; string++;
/* Parse ACE account sid */ /* Parse ACE account sid */
if (parse_sid( string, ace ? (SID *)&ace->SidStart : NULL, &sidlen )) if (!parse_sid( string, &string, ace ? (SID *)&ace->SidStart : NULL, &sidlen ))
{ goto err;
while (*string && *string != ')')
string++; while (*string == ' ')
} string++;
if (*string != ')') if (*string != ')')
goto err; goto err;
...@@ -1095,7 +1107,7 @@ static BOOL parse_sd( const WCHAR *string, SECURITY_DESCRIPTOR_RELATIVE *sd, DWO ...@@ -1095,7 +1107,7 @@ static BOOL parse_sd( const WCHAR *string, SECURITY_DESCRIPTOR_RELATIVE *sd, DWO
{ {
DWORD bytes; DWORD bytes;
if (!parse_sid( tok, (SID *)next, &bytes )) if (!parse_sid( tok, NULL, (SID *)next, &bytes ))
goto out; goto out;
if (sd) if (sd)
...@@ -1113,7 +1125,7 @@ static BOOL parse_sd( const WCHAR *string, SECURITY_DESCRIPTOR_RELATIVE *sd, DWO ...@@ -1113,7 +1125,7 @@ static BOOL parse_sd( const WCHAR *string, SECURITY_DESCRIPTOR_RELATIVE *sd, DWO
{ {
DWORD bytes; DWORD bytes;
if (!parse_sid( tok, (SID *)next, &bytes )) if (!parse_sid( tok, NULL, (SID *)next, &bytes ))
goto out; goto out;
if (sd) if (sd)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment