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

Commitcfc40d3

Browse files
committed
Introduce safer encoding and decoding routines for base64.c
This is a follow-up refactoring after09ec55b andb674211, which hasproved that the encoding and decoding routines used by SCRAM have apoor interface when it comes to check after buffer overflows. This addsan extra argument in the shape of the length of the result buffer foreach routine, which is used for overflow checks when encoding ordecoding an input string. The original idea comes from Tom Lane.As a result of that, the encoding routine can now fail, so all itscallers are adjusted to generate proper error messages in case ofproblems.On failure, the result buffer gets zeroed.Author: Michael PaquierReviewed-by: Daniel GustafssonDiscussion:https://postgr.es/m/20190623132535.GB1628@paquier.xyz
1 parentd5ab9a8 commitcfc40d3

File tree

5 files changed

+210
-46
lines changed

5 files changed

+210
-46
lines changed

‎src/backend/libpq/auth-scram.c

Lines changed: 55 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,11 @@ scram_verify_plain_password(const char *username, const char *password,
510510
return false;
511511
}
512512

513-
salt=palloc(pg_b64_dec_len(strlen(encoded_salt)));
514-
saltlen=pg_b64_decode(encoded_salt,strlen(encoded_salt),salt);
515-
if (saltlen==-1)
513+
saltlen=pg_b64_dec_len(strlen(encoded_salt));
514+
salt=palloc(saltlen);
515+
saltlen=pg_b64_decode(encoded_salt,strlen(encoded_salt),salt,
516+
saltlen);
517+
if (saltlen<0)
516518
{
517519
ereport(LOG,
518520
(errmsg("invalid SCRAM verifier for user \"%s\"",username)));
@@ -596,26 +598,29 @@ parse_scram_verifier(const char *verifier, int *iterations, char **salt,
596598
* Verify that the salt is in Base64-encoded format, by decoding it,
597599
* although we return the encoded version to the caller.
598600
*/
599-
decoded_salt_buf=palloc(pg_b64_dec_len(strlen(salt_str)));
601+
decoded_len=pg_b64_dec_len(strlen(salt_str));
602+
decoded_salt_buf=palloc(decoded_len);
600603
decoded_len=pg_b64_decode(salt_str,strlen(salt_str),
601-
decoded_salt_buf);
604+
decoded_salt_buf,decoded_len);
602605
if (decoded_len<0)
603606
gotoinvalid_verifier;
604607
*salt=pstrdup(salt_str);
605608

606609
/*
607610
* Decode StoredKey and ServerKey.
608611
*/
609-
decoded_stored_buf=palloc(pg_b64_dec_len(strlen(storedkey_str)));
612+
decoded_len=pg_b64_dec_len(strlen(storedkey_str));
613+
decoded_stored_buf=palloc(decoded_len);
610614
decoded_len=pg_b64_decode(storedkey_str,strlen(storedkey_str),
611-
decoded_stored_buf);
615+
decoded_stored_buf,decoded_len);
612616
if (decoded_len!=SCRAM_KEY_LEN)
613617
gotoinvalid_verifier;
614618
memcpy(stored_key,decoded_stored_buf,SCRAM_KEY_LEN);
615619

616-
decoded_server_buf=palloc(pg_b64_dec_len(strlen(serverkey_str)));
620+
decoded_len=pg_b64_dec_len(strlen(serverkey_str));
621+
decoded_server_buf=palloc(decoded_len);
617622
decoded_len=pg_b64_decode(serverkey_str,strlen(serverkey_str),
618-
decoded_server_buf);
623+
decoded_server_buf,decoded_len);
619624
if (decoded_len!=SCRAM_KEY_LEN)
620625
gotoinvalid_verifier;
621626
memcpy(server_key,decoded_server_buf,SCRAM_KEY_LEN);
@@ -649,8 +654,20 @@ mock_scram_verifier(const char *username, int *iterations, char **salt,
649654
/* Generate deterministic salt */
650655
raw_salt=scram_mock_salt(username);
651656

652-
encoded_salt= (char*)palloc(pg_b64_enc_len(SCRAM_DEFAULT_SALT_LEN)+1);
653-
encoded_len=pg_b64_encode(raw_salt,SCRAM_DEFAULT_SALT_LEN,encoded_salt);
657+
encoded_len=pg_b64_enc_len(SCRAM_DEFAULT_SALT_LEN);
658+
/* don't forget the zero-terminator */
659+
encoded_salt= (char*)palloc(encoded_len+1);
660+
encoded_len=pg_b64_encode(raw_salt,SCRAM_DEFAULT_SALT_LEN,encoded_salt,
661+
encoded_len);
662+
663+
/*
664+
* Note that we cannot reveal any information to an attacker here so the
665+
* error message needs to remain generic. This should never fail anyway
666+
* as the salt generated for mock authentication uses the cluster's nonce
667+
* value.
668+
*/
669+
if (encoded_len<0)
670+
elog(ERROR,"could not encode salt");
654671
encoded_salt[encoded_len]='\0';
655672

656673
*salt=encoded_salt;
@@ -1144,8 +1161,15 @@ build_server_first_message(scram_state *state)
11441161
(errcode(ERRCODE_INTERNAL_ERROR),
11451162
errmsg("could not generate random nonce")));
11461163

1147-
state->server_nonce=palloc(pg_b64_enc_len(SCRAM_RAW_NONCE_LEN)+1);
1148-
encoded_len=pg_b64_encode(raw_nonce,SCRAM_RAW_NONCE_LEN,state->server_nonce);
1164+
encoded_len=pg_b64_enc_len(SCRAM_RAW_NONCE_LEN);
1165+
/* don't forget the zero-terminator */
1166+
state->server_nonce=palloc(encoded_len+1);
1167+
encoded_len=pg_b64_encode(raw_nonce,SCRAM_RAW_NONCE_LEN,
1168+
state->server_nonce,encoded_len);
1169+
if (encoded_len<0)
1170+
ereport(ERROR,
1171+
(errcode(ERRCODE_INTERNAL_ERROR),
1172+
errmsg("could not encode random nonce")));
11491173
state->server_nonce[encoded_len]='\0';
11501174

11511175
state->server_first_message=
@@ -1170,6 +1194,7 @@ read_client_final_message(scram_state *state, const char *input)
11701194
*proof;
11711195
char*p;
11721196
char*client_proof;
1197+
intclient_proof_len;
11731198

11741199
begin=p=pstrdup(input);
11751200

@@ -1234,9 +1259,13 @@ read_client_final_message(scram_state *state, const char *input)
12341259
snprintf(cbind_input,cbind_input_len,"p=tls-server-end-point,,");
12351260
memcpy(cbind_input+cbind_header_len,cbind_data,cbind_data_len);
12361261

1237-
b64_message=palloc(pg_b64_enc_len(cbind_input_len)+1);
1262+
b64_message_len=pg_b64_enc_len(cbind_input_len);
1263+
/* don't forget the zero-terminator */
1264+
b64_message=palloc(b64_message_len+1);
12381265
b64_message_len=pg_b64_encode(cbind_input,cbind_input_len,
1239-
b64_message);
1266+
b64_message,b64_message_len);
1267+
if (b64_message_len<0)
1268+
elog(ERROR,"could not encode channel binding data");
12401269
b64_message[b64_message_len]='\0';
12411270

12421271
/*
@@ -1276,8 +1305,10 @@ read_client_final_message(scram_state *state, const char *input)
12761305
value=read_any_attr(&p,&attr);
12771306
}while (attr!='p');
12781307

1279-
client_proof=palloc(pg_b64_dec_len(strlen(value)));
1280-
if (pg_b64_decode(value,strlen(value),client_proof)!=SCRAM_KEY_LEN)
1308+
client_proof_len=pg_b64_dec_len(strlen(value));
1309+
client_proof=palloc(client_proof_len);
1310+
if (pg_b64_decode(value,strlen(value),client_proof,
1311+
client_proof_len)!=SCRAM_KEY_LEN)
12811312
ereport(ERROR,
12821313
(errcode(ERRCODE_PROTOCOL_VIOLATION),
12831314
errmsg("malformed SCRAM message"),
@@ -1322,9 +1353,14 @@ build_server_final_message(scram_state *state)
13221353
strlen(state->client_final_message_without_proof));
13231354
scram_HMAC_final(ServerSignature,&ctx);
13241355

1325-
server_signature_base64=palloc(pg_b64_enc_len(SCRAM_KEY_LEN)+1);
1356+
siglen=pg_b64_enc_len(SCRAM_KEY_LEN);
1357+
/* don't forget the zero-terminator */
1358+
server_signature_base64=palloc(siglen+1);
13261359
siglen=pg_b64_encode((constchar*)ServerSignature,
1327-
SCRAM_KEY_LEN,server_signature_base64);
1360+
SCRAM_KEY_LEN,server_signature_base64,
1361+
siglen);
1362+
if (siglen<0)
1363+
elog(ERROR,"could not encode server signature");
13281364
server_signature_base64[siglen]='\0';
13291365

13301366
/*------

‎src/common/base64.c

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,11 @@ static const int8 b64lookup[128] = {
4242
* pg_b64_encode
4343
*
4444
* Encode into base64 the given string. Returns the length of the encoded
45-
* string.
45+
* string, and -1 in the event of an error with the result buffer zeroed
46+
* for safety.
4647
*/
4748
int
48-
pg_b64_encode(constchar*src,intlen,char*dst)
49+
pg_b64_encode(constchar*src,intlen,char*dst,intdstlen)
4950
{
5051
char*p;
5152
constchar*s,
@@ -65,6 +66,13 @@ pg_b64_encode(const char *src, int len, char *dst)
6566
/* write it out */
6667
if (pos<0)
6768
{
69+
/*
70+
* Leave if there is an overflow in the area allocated for the
71+
* encoded string.
72+
*/
73+
if ((p-dst+4)>dstlen)
74+
gotoerror;
75+
6876
*p++=_base64[(buf >>18)&0x3f];
6977
*p++=_base64[(buf >>12)&0x3f];
7078
*p++=_base64[(buf >>6)&0x3f];
@@ -76,23 +84,36 @@ pg_b64_encode(const char *src, int len, char *dst)
7684
}
7785
if (pos!=2)
7886
{
87+
/*
88+
* Leave if there is an overflow in the area allocated for the encoded
89+
* string.
90+
*/
91+
if ((p-dst+4)>dstlen)
92+
gotoerror;
93+
7994
*p++=_base64[(buf >>18)&0x3f];
8095
*p++=_base64[(buf >>12)&0x3f];
8196
*p++= (pos==0) ?_base64[(buf >>6)&0x3f] :'=';
8297
*p++='=';
8398
}
8499

100+
Assert((p-dst) <=dstlen);
85101
returnp-dst;
102+
103+
error:
104+
memset(dst,0,dstlen);
105+
return-1;
86106
}
87107

88108
/*
89109
* pg_b64_decode
90110
*
91111
* Decode the given base64 string. Returns the length of the decoded
92-
* string on success, and -1 in the event of an error.
112+
* string on success, and -1 in the event of an error with the result
113+
* buffer zeroed for safety.
93114
*/
94115
int
95-
pg_b64_decode(constchar*src,intlen,char*dst)
116+
pg_b64_decode(constchar*src,intlen,char*dst,intdstlen)
96117
{
97118
constchar*srcend=src+len,
98119
*s=src;
@@ -109,7 +130,7 @@ pg_b64_decode(const char *src, int len, char *dst)
109130

110131
/* Leave if a whitespace is found */
111132
if (c==' '||c=='\t'||c=='\n'||c=='\r')
112-
return-1;
133+
gotoerror;
113134

114135
if (c=='=')
115136
{
@@ -126,7 +147,7 @@ pg_b64_decode(const char *src, int len, char *dst)
126147
* Unexpected "=" character found while decoding base64
127148
* sequence.
128149
*/
129-
return-1;
150+
gotoerror;
130151
}
131152
}
132153
b=0;
@@ -139,19 +160,36 @@ pg_b64_decode(const char *src, int len, char *dst)
139160
if (b<0)
140161
{
141162
/* invalid symbol found */
142-
return-1;
163+
gotoerror;
143164
}
144165
}
145166
/* add it to buffer */
146167
buf= (buf <<6)+b;
147168
pos++;
148169
if (pos==4)
149170
{
171+
/*
172+
* Leave if there is an overflow in the area allocated for the
173+
* decoded string.
174+
*/
175+
if ((p-dst+1)>dstlen)
176+
gotoerror;
150177
*p++= (buf >>16)&255;
178+
151179
if (end==0||end>1)
180+
{
181+
/* overflow check */
182+
if ((p-dst+1)>dstlen)
183+
gotoerror;
152184
*p++= (buf >>8)&255;
185+
}
153186
if (end==0||end>2)
187+
{
188+
/* overflow check */
189+
if ((p-dst+1)>dstlen)
190+
gotoerror;
154191
*p++=buf&255;
192+
}
155193
buf=0;
156194
pos=0;
157195
}
@@ -163,10 +201,15 @@ pg_b64_decode(const char *src, int len, char *dst)
163201
* base64 end sequence is invalid. Input data is missing padding, is
164202
* truncated or is otherwise corrupted.
165203
*/
166-
return-1;
204+
gotoerror;
167205
}
168206

207+
Assert((p-dst) <=dstlen);
169208
returnp-dst;
209+
210+
error:
211+
memset(dst,0,dstlen);
212+
return-1;
170213
}
171214

172215
/*

‎src/common/scram-common.c

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,10 @@ scram_build_verifier(const char *salt, int saltlen, int iterations,
198198
char*result;
199199
char*p;
200200
intmaxlen;
201+
intencoded_salt_len;
202+
intencoded_stored_len;
203+
intencoded_server_len;
204+
intencoded_result;
201205

202206
if (iterations <=0)
203207
iterations=SCRAM_DEFAULT_ITERATIONS;
@@ -215,11 +219,15 @@ scram_build_verifier(const char *salt, int saltlen, int iterations,
215219
* SCRAM-SHA-256$<iteration count>:<salt>$<StoredKey>:<ServerKey>
216220
*----------
217221
*/
222+
encoded_salt_len=pg_b64_enc_len(saltlen);
223+
encoded_stored_len=pg_b64_enc_len(SCRAM_KEY_LEN);
224+
encoded_server_len=pg_b64_enc_len(SCRAM_KEY_LEN);
225+
218226
maxlen=strlen("SCRAM-SHA-256")+1
219227
+10+1/* iteration count */
220-
+pg_b64_enc_len(saltlen)+1/* Base64-encoded salt */
221-
+pg_b64_enc_len(SCRAM_KEY_LEN)+1/* Base64-encoded StoredKey */
222-
+pg_b64_enc_len(SCRAM_KEY_LEN)+1;/* Base64-encoded ServerKey */
228+
+encoded_salt_len+1/* Base64-encoded salt */
229+
+encoded_stored_len+1/* Base64-encoded StoredKey */
230+
+encoded_server_len+1;/* Base64-encoded ServerKey */
223231

224232
#ifdefFRONTEND
225233
result=malloc(maxlen);
@@ -231,11 +239,50 @@ scram_build_verifier(const char *salt, int saltlen, int iterations,
231239

232240
p=result+sprintf(result,"SCRAM-SHA-256$%d:",iterations);
233241

234-
p+=pg_b64_encode(salt,saltlen,p);
242+
/* salt */
243+
encoded_result=pg_b64_encode(salt,saltlen,p,encoded_salt_len);
244+
if (encoded_result<0)
245+
{
246+
#ifdefFRONTEND
247+
free(result);
248+
returnNULL;
249+
#else
250+
elog(ERROR,"could not encode salt");
251+
#endif
252+
}
253+
p+=encoded_result;
235254
*(p++)='$';
236-
p+=pg_b64_encode((char*)stored_key,SCRAM_KEY_LEN,p);
255+
256+
/* stored key */
257+
encoded_result=pg_b64_encode((char*)stored_key,SCRAM_KEY_LEN,p,
258+
encoded_stored_len);
259+
if (encoded_result<0)
260+
{
261+
#ifdefFRONTEND
262+
free(result);
263+
returnNULL;
264+
#else
265+
elog(ERROR,"could not encode stored key");
266+
#endif
267+
}
268+
269+
p+=encoded_result;
237270
*(p++)=':';
238-
p+=pg_b64_encode((char*)server_key,SCRAM_KEY_LEN,p);
271+
272+
/* server key */
273+
encoded_result=pg_b64_encode((char*)server_key,SCRAM_KEY_LEN,p,
274+
encoded_server_len);
275+
if (encoded_result<0)
276+
{
277+
#ifdefFRONTEND
278+
free(result);
279+
returnNULL;
280+
#else
281+
elog(ERROR,"could not encode server key");
282+
#endif
283+
}
284+
285+
p+=encoded_result;
239286
*(p++)='\0';
240287

241288
Assert(p-result <=maxlen);

‎src/include/common/base64.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
#defineBASE64_H
1212

1313
/* base 64 */
14-
externintpg_b64_encode(constchar*src,intlen,char*dst);
15-
externintpg_b64_decode(constchar*src,intlen,char*dst);
14+
externintpg_b64_encode(constchar*src,intlen,char*dst,intdstlen);
15+
externintpg_b64_decode(constchar*src,intlen,char*dst,intdstlen);
1616
externintpg_b64_enc_len(intsrclen);
1717
externintpg_b64_dec_len(intsrclen);
1818

0 commit comments

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