Skip to content

Commit

Permalink
Merge pull request #2336 from dscho/program-data-config-owned-by-spec…
Browse files Browse the repository at this point in the history
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
  • Loading branch information
dscho committed Oct 2, 2019
2 parents 8c5225c + 8f8ffd9 commit d01cfeb
Showing 1 changed file with 113 additions and 48 deletions.
161 changes: 113 additions & 48 deletions compat/mingw.c
Expand Up @@ -3299,6 +3299,97 @@ int uname(struct utsname *buf)
return 0; return 0;
} }


/*
* Determines whether the SID refers to an administrator or the current user.
*
* For convenience, the `info` parameter allows avoiding multiple calls to
* `OpenProcessToken()` if this function is called more than once. The initial
* value of `*info` is expected to be `NULL`, and it needs to be released via
* `free()` after the last call to this function.
*
* Returns 0 if the SID indicates a dubious owner of system files, otherwise 1.
*/
static int is_valid_system_file_owner(PSID sid, TOKEN_USER **info)
{
HANDLE token;
DWORD len;
char builtin_administrators_sid[SECURITY_MAX_SID_SIZE];

if (IsWellKnownSid(sid, WinBuiltinAdministratorsSid) ||
IsWellKnownSid(sid, WinLocalSystemSid))
return 1;

/* Obtain current user's SID */
if (!*info &&
OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token)) {
if (!GetTokenInformation(token, TokenUser, NULL, 0, &len)) {
*info = xmalloc((size_t)len);
if (!GetTokenInformation(token, TokenUser, *info, len,
&len))
FREE_AND_NULL(*info);
}
CloseHandle(token);
}

if (*info && EqualSid(sid, (*info)->User.Sid))
return 1;

/* Is the owner at least a member of BUILTIN\Administrators? */
len = ARRAY_SIZE(builtin_administrators_sid);
if (CreateWellKnownSid(WinBuiltinAdministratorsSid, NULL,
builtin_administrators_sid, &len)) {
wchar_t name[256], domain[256];
DWORD name_size = ARRAY_SIZE(name);
DWORD domain_size = ARRAY_SIZE(domain);
SID_NAME_USE type;
PSID *members;
DWORD dummy, i;
/*
* We avoid including the `lm.h` header and linking to
* `netapi32.dll` directly, in favor of lazy-loading that DLL
* when, and _only_ when, needed.
*/
DECLARE_PROC_ADDR(netapi32.dll, DWORD,
NetLocalGroupGetMembers, LPCWSTR,
LPCWSTR, DWORD, LPVOID, DWORD,
LPDWORD, LPDWORD, PDWORD_PTR);
DECLARE_PROC_ADDR(netapi32.dll, DWORD,
NetApiBufferFree, LPVOID);

if (LookupAccountSidW(NULL, builtin_administrators_sid,
name, &name_size, domain, &domain_size,
&type) &&
INIT_PROC_ADDR(NetLocalGroupGetMembers) &&
/*
* Technically, `NetLocalGroupGetMembers()` wants to assign
* an array of type `LOCALGROUP_MEMBERS_INFO_0`, which
* however contains only one field of type `PSID`,
* therefore we can pretend that it is an array over the
* type `PSID`.
*
* Also, we simply ignore the condition where
* `ERROR_MORE_DATA` is returned; This should not happen
* anyway, as we are passing `-1` as `prefmaxlen`
* parameter, which is equivalent to the constant
* `MAX_PREFERRED_LENGTH`.
*/
!NetLocalGroupGetMembers(NULL, name, 0, &members, -1,
&len, &dummy, NULL)) {
for (i = 0; i < len; i++)
if (EqualSid(sid, members[i]))
break;

if (INIT_PROC_ADDR(NetApiBufferFree))
NetApiBufferFree(members);

/* Did we find the `sid` in the members? */
return i < len;
}
}

return 0;
}

/* /*
* Verify that the file in question is owned by an administrator or system * Verify that the file in question is owned by an administrator or system
* account, or at least by the current user. * account, or at least by the current user.
Expand All @@ -3309,11 +3400,10 @@ int uname(struct utsname *buf)
static int validate_system_file_ownership(const char *path) static int validate_system_file_ownership(const char *path)
{ {
WCHAR wpath[MAX_LONG_PATH]; WCHAR wpath[MAX_LONG_PATH];
PSID owner_sid = NULL; PSID owner_sid = NULL, problem_sid = NULL;
PSECURITY_DESCRIPTOR descriptor = NULL; PSECURITY_DESCRIPTOR descriptor = NULL;
HANDLE token;
TOKEN_USER* info = NULL; TOKEN_USER* info = NULL;
DWORD err, len; DWORD err;
int ret; int ret;


if (xutftowcs_long_path(wpath, path) < 0) if (xutftowcs_long_path(wpath, path) < 0)
Expand All @@ -3325,63 +3415,37 @@ static int validate_system_file_ownership(const char *path)
&owner_sid, NULL, NULL, NULL, &descriptor); &owner_sid, NULL, NULL, NULL, &descriptor);


/* if the file does not exist, it does not have a valid owner */ /* if the file does not exist, it does not have a valid owner */
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) { if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND)
ret = 0; ret = 0;
owner_sid = NULL; else if (err != ERROR_SUCCESS)
goto finish_validation;
}

if (err != ERROR_SUCCESS) {
ret = error(_("failed to validate '%s' (%ld)"), path, err); ret = error(_("failed to validate '%s' (%ld)"), path, err);
owner_sid = NULL; else if (!IsValidSid(owner_sid))
goto finish_validation;
}

if (!IsValidSid(owner_sid)) {
ret = error(_("invalid owner: '%s'"), path); ret = error(_("invalid owner: '%s'"), path);
goto finish_validation; else if (is_valid_system_file_owner(owner_sid, &info))
}

if (IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) ||
IsWellKnownSid(owner_sid, WinLocalSystemSid)) {
ret = 1; ret = 1;
goto finish_validation;
}

/* Obtain current user's SID */
if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token) &&
!GetTokenInformation(token, TokenUser, NULL, 0, &len)) {
info = xmalloc((size_t)len);
if (!GetTokenInformation(token, TokenUser, info, len, &len))
FREE_AND_NULL(info);
}

if (!info)
ret = 0;
else { else {
ret = EqualSid(owner_sid, info->User.Sid) ? 1 : 0; ret = 0;
free(info); problem_sid = owner_sid;
} }


finish_validation: if (!ret && problem_sid) {
if (!ret && owner_sid) {
#define MAX_NAME_OR_DOMAIN 256 #define MAX_NAME_OR_DOMAIN 256
wchar_t owner_name[MAX_NAME_OR_DOMAIN]; wchar_t name[MAX_NAME_OR_DOMAIN];
wchar_t owner_domain[MAX_NAME_OR_DOMAIN]; wchar_t domain[MAX_NAME_OR_DOMAIN];
wchar_t *p = NULL; wchar_t *p = NULL;
DWORD size = MAX_NAME_OR_DOMAIN; DWORD size = MAX_NAME_OR_DOMAIN;
SID_NAME_USE type; SID_NAME_USE type;
char name[3 * MAX_NAME_OR_DOMAIN + 1]; char utf[3 * MAX_NAME_OR_DOMAIN + 1];


if (!LookupAccountSidW(NULL, owner_sid, owner_name, &size, if (!LookupAccountSidW(NULL, problem_sid, name, &size,
owner_domain, &size, &type) || domain, &size, &type) ||
xwcstoutf(name, owner_name, ARRAY_SIZE(name)) < 0) { xwcstoutf(utf, name, ARRAY_SIZE(utf)) < 0) {
if (!ConvertSidToStringSidW(owner_sid, &p)) if (!ConvertSidToStringSidW(problem_sid, &p))
strlcpy(name, "(unknown)", ARRAY_SIZE(name)); strlcpy(utf, "(unknown)", ARRAY_SIZE(utf));
else { else {
if (xwcstoutf(name, p, ARRAY_SIZE(name)) < 0) if (xwcstoutf(utf, p, ARRAY_SIZE(utf)) < 0)
strlcpy(name, "(some user)", strlcpy(utf, "(some user)",
ARRAY_SIZE(name)); ARRAY_SIZE(utf));
LocalFree(p); LocalFree(p);
} }
} }
Expand All @@ -3390,11 +3454,12 @@ static int validate_system_file_ownership(const char *path)
"For security reasons, it is therefore ignored.\n" "For security reasons, it is therefore ignored.\n"
"To fix this, please transfer ownership to an " "To fix this, please transfer ownership to an "
"admininstrator."), "admininstrator."),
path, name); path, utf);
} }


if (descriptor) if (descriptor)
LocalFree(descriptor); LocalFree(descriptor);
free(info);


return ret; return ret;
} }
Expand Down

0 comments on commit d01cfeb

Please sign in to comment.