Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit126cdaf

Browse files
committed
Don't assume GSSAPI result strings are null-terminated.
Our uses of gss_display_status() and gss_display_name() assumedthat the gss_buffer_desc strings returned by those functions arenull-terminated. It appears that they generally are, given thelack of field complaints up to now. However, the availabledocumentation does not promise this, and some man pagesfor gss_display_status() show examples that rely on thegss_buffer_desc.length field instead of expecting nulltermination. Also, we now have a report that on someimplementations, clang's address sanitizer is of the opinionthat the byte after the specified length is undefined.Hence, change the code to rely on the length field instead.This might well be cosmetic rather than fixing any real bug, butit's hard to be sure, so back-patch to all supported branches.While here, also back-patch the v12 changes that made pg_GSS_errordeal honestly with multiple messages available fromgss_display_status.Per report from Sudheer H R.Discussion:https://postgr.es/m/5372B6D4-8276-42C0-B8FB-BD0918826FC3@tekenlight.com
1 parent4a05406 commit126cdaf

File tree

3 files changed

+26
-15
lines changed

3 files changed

+26
-15
lines changed

‎src/backend/libpq/auth.c

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,6 +1213,7 @@ pg_GSS_checkauth(Port *port)
12131213
min_stat,
12141214
lmin_s;
12151215
gss_buffer_descgbuf;
1216+
char*princ;
12161217

12171218
/*
12181219
* Get the name of the user that authenticated, and compare it to the pg
@@ -1226,6 +1227,15 @@ pg_GSS_checkauth(Port *port)
12261227
returnSTATUS_ERROR;
12271228
}
12281229

1230+
/*
1231+
* gbuf.value might not be null-terminated, so turn it into a regular
1232+
* null-terminated string.
1233+
*/
1234+
princ=palloc(gbuf.length+1);
1235+
memcpy(princ,gbuf.value,gbuf.length);
1236+
princ[gbuf.length]='\0';
1237+
gss_release_buffer(&lmin_s,&gbuf);
1238+
12291239
/*
12301240
* Copy the original name of the authenticated principal into our backend
12311241
* memory for display later.
@@ -1234,15 +1244,15 @@ pg_GSS_checkauth(Port *port)
12341244
* waiting for the usermap check below, because authentication has already
12351245
* succeeded and we want the log file to reflect that.
12361246
*/
1237-
port->gss->princ=MemoryContextStrdup(TopMemoryContext,gbuf.value);
1238-
set_authn_id(port,gbuf.value);
1247+
port->gss->princ=MemoryContextStrdup(TopMemoryContext,princ);
1248+
set_authn_id(port,princ);
12391249

12401250
/*
12411251
* Split the username at the realm separator
12421252
*/
1243-
if (strchr(gbuf.value,'@'))
1253+
if (strchr(princ,'@'))
12441254
{
1245-
char*cp=strchr(gbuf.value,'@');
1255+
char*cp=strchr(princ,'@');
12461256

12471257
/*
12481258
* If we are not going to include the realm in the username that is
@@ -1269,7 +1279,7 @@ pg_GSS_checkauth(Port *port)
12691279
elog(DEBUG2,
12701280
"GSSAPI realm (%s) and configured realm (%s) don't match",
12711281
cp,port->hba->krb_realm);
1272-
gss_release_buffer(&lmin_s,&gbuf);
1282+
pfree(princ);
12731283
returnSTATUS_ERROR;
12741284
}
12751285
}
@@ -1278,15 +1288,14 @@ pg_GSS_checkauth(Port *port)
12781288
{
12791289
elog(DEBUG2,
12801290
"GSSAPI did not return realm but realm matching was requested");
1281-
1282-
gss_release_buffer(&lmin_s,&gbuf);
1291+
pfree(princ);
12831292
returnSTATUS_ERROR;
12841293
}
12851294

1286-
ret=check_usermap(port->hba->usermap,port->user_name,gbuf.value,
1295+
ret=check_usermap(port->hba->usermap,port->user_name,princ,
12871296
pg_krb_caseins_users);
12881297

1289-
gss_release_buffer(&lmin_s,&gbuf);
1298+
pfree(princ);
12901299

12911300
returnret;
12921301
}

‎src/backend/libpq/be-gssapi-common.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ pg_GSS_error_int(char *s, size_t len, OM_uint32 stat, int type)
2929
OM_uint32lmin_s,
3030
msg_ctx=0;
3131

32-
s[0]='\0';/* just in case gss_display_status fails */
33-
3432
do
3533
{
3634
if (gss_display_status(&lmin_s,stat,type,GSS_C_NO_OID,
@@ -43,16 +41,19 @@ pg_GSS_error_int(char *s, size_t len, OM_uint32 stat, int type)
4341
i++;
4442
}
4543
if (i<len)
46-
strlcpy(s+i,gmsg.value,len-i);
44+
memcpy(s+i,gmsg.value,Min(len-i,gmsg.length));
4745
i+=gmsg.length;
4846
gss_release_buffer(&lmin_s,&gmsg);
4947
}
5048
while (msg_ctx);
5149

52-
if (i >=len)
50+
/* add nul termination */
51+
if (i<len)
52+
s[i]='\0';
53+
else
5354
{
5455
elog(COMMERROR,"incomplete GSS error report");
55-
s[len-1]='\0';/* ensure string is nul-terminated */
56+
s[len-1]='\0';
5657
}
5758
}
5859

‎src/interfaces/libpq/fe-gssapi-common.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ pg_GSS_error_int(PQExpBuffer str, OM_uint32 stat, int type)
3434
if (gss_display_status(&lmin_s,stat,type,GSS_C_NO_OID,
3535
&msg_ctx,&lmsg)!=GSS_S_COMPLETE)
3636
break;
37-
appendPQExpBuffer(str," %s", (char*)lmsg.value);
37+
appendPQExpBufferChar(str,' ');
38+
appendBinaryPQExpBuffer(str,lmsg.value,lmsg.length);
3839
gss_release_buffer(&lmin_s,&lmsg);
3940
}while (msg_ctx);
4041
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp