From 7a0b1d4d2d11ca2cb920846a8be9caf61792167a Mon Sep 17 00:00:00 2001 From: Marc Becker Date: Wed, 14 Oct 2026 20:26:48 +0200 Subject: [PATCH] wincred: refactor credential blob processing Implementation for splitting secret blob into separate lines and parsing those into key/value pairs has a couple of issues: - invalid size limit to `wcsncpy_s` (missed during d22a4884) - inconsistent detection of line separator (matches _CR_ and _LF_) - failing to advance position after initial password line - deep code nesting in loop body. Line end detection needs to be deterministic and code can be split up into smaller functional blocks. Create new methods for processing the matching credential and each supported key/value pair of a multi-line secret. Reduce variable scope and nesting level. Use early continue/return to improve code readability. Use `wmemchr` to reliably detect wide-character linefeed in immutable blob data without need to create a further copy. Signed-off-by: Marc Becker --- .../wincred/git-credential-wincred.c | 124 ++++++++++++------ 1 file changed, 85 insertions(+), 39 deletions(-) diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index 22eb27ca31dea0..e2b40ef8c82cf7 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -6,6 +6,7 @@ #include #include #include +#include #include /* common helpers */ @@ -69,6 +70,68 @@ static void write_item(const char *what, LPCWSTR wbuf, int wlen) free(buf); } +/* + * Write known credential item. + */ +static void write_credential_item(LPCWSTR data, int wlen) +{ + static const LPCWSTR refresh_token = L"oauth_refresh_token"; + LPCWSTR value; + DWORD klen; + DWORD vlen; + + /* find key/value separator for credential item */ + if ((value = wmemchr(data, L'=', wlen)) == NULL) + return; + klen = value++ - data; + vlen = wlen - klen - 1; + + /* write items known to git credential protocol */ + if (klen == wcslen(refresh_token) && wmemcmp(data, refresh_token, klen) == 0) + write_item("oauth_refresh_token", value, vlen); +} + +/* + * Write single credential block + * consisting of password and further (accepted) credential items. + */ +static void write_credential_secret(LPCWSTR blob, DWORD wlen) +{ + LPCWSTR end; + DWORD length; + + /* check if content is single line (retain trailing carriage return) */ + if ((end = wmemchr(blob, L'\n', wlen)) == NULL) { + write_item("password", blob, wlen); + return; + } + /* determine current line length and remaining data size */ + length = end++ - blob; + wlen -= length + 1; + + /* trim carriage return at line end */ + if (length && blob[length - 1] == L'\r') + --length; + write_item("password", blob, length); + blob = end; + + /* find line separator */ + while ((end = wmemchr(blob, L'\n', wlen)) != NULL) { + length = end++ - blob; + wlen -= length + 1; + + /* trim carriage return at line end */ + if (length && blob[length - 1] == L'\r') + --length; + + /* write item and advance blob position */ + write_credential_item(blob, length); + blob = end; + } + /* process final line (retain trailing carriage return) */ + write_credential_item(blob, wlen); +} + /* * Match an (optional) expected string and a delimiter in the target string, * consuming the matched text by updating the target pointer. @@ -148,51 +211,34 @@ static void get_credential(void) { CREDENTIALW **creds; DWORD num_creds; - int i; - CREDENTIAL_ATTRIBUTEW *attr; - WCHAR *secret; - WCHAR *line; - WCHAR *remaining_lines; - WCHAR *part; - WCHAR *remaining_parts; if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds)) return; - /* search for the first credential that matches username */ - for (i = 0; i < num_creds; ++i) - if (match_cred(creds[i], 0)) { - write_item("username", creds[i]->UserName, - creds[i]->UserName ? wcslen(creds[i]->UserName) : 0); - if (creds[i]->CredentialBlobSize > 0) { - secret = xmalloc(creds[i]->CredentialBlobSize + sizeof(WCHAR)); - wcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR)); - line = wcstok_s(secret, L"\r\n", &remaining_lines); - write_item("password", line, line ? wcslen(line) : 0); - while(line != NULL) { - part = wcstok_s(line, L"=", &remaining_parts); - if (!wcscmp(part, L"oauth_refresh_token")) { - write_item("oauth_refresh_token", remaining_parts, remaining_parts ? wcslen(remaining_parts) : 0); - } - line = wcstok_s(NULL, L"\r\n", &remaining_lines); - } - free(secret); - } else { - write_item("password", - (LPCWSTR)creds[i]->CredentialBlob, - creds[i]->CredentialBlobSize / sizeof(WCHAR)); - } - for (int j = 0; j < creds[i]->AttributeCount; j++) { - attr = creds[i]->Attributes + j; - if (!wcscmp(attr->Keyword, L"git_password_expiry_utc")) { - write_item("password_expiry_utc", (LPCWSTR)attr->Value, - attr->ValueSize / sizeof(WCHAR)); - break; - } + /* search for the first credential that matches target and username */ + for (int i = 0; i < num_creds; ++i) { + LPCWSTR username; + LPCWSTR blob; + + if (!match_cred(creds[i], 0)) + continue; + + username = creds[i]->UserName; + write_item("username", username, username ? wcslen(username) : 0); + + blob = (LPCWSTR)creds[i]->CredentialBlob; + write_credential_secret(blob, creds[i]->CredentialBlobSize / sizeof(*blob)); + + for (int j = 0; j < creds[i]->AttributeCount; j++) { + CREDENTIAL_ATTRIBUTEW *attr = creds[i]->Attributes + j; + + if (!wcscmp(attr->Keyword, L"git_password_expiry_utc")) { + write_item("password_expiry_utc", (LPCWSTR)attr->Value, attr->ValueSize / sizeof(WCHAR)); + break; } - break; } - + break; + } CredFree(creds); }