Skip to content

Commit f0fa514

Browse files
olszomalolszomal
committed
Fix heap corruption in PE page hash calculation
1 parent 09d3312 commit f0fa514

File tree

1 file changed

+125
-33
lines changed

1 file changed

+125
-33
lines changed

‎pe.c‎

Lines changed: 125 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -916,15 +916,26 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype)
916916
uint16_t nsections, opthdr_size;
917917
uint32_t alignment, pagesize, hdrsize;
918918
uint32_t rs, ro, l, lastpos = 0;
919-
int pphlen, phlen, i, pi = 1;
920-
size_t written;
921-
u_char *res, *zeroes;
919+
int mdlen, pphlen, phlen, i, pi = 1;
920+
size_t written, off, sect_off, sect_tbl, need;
921+
u_char *res = NULL, *zeroes = NULL;
922922
char *sections;
923923
const EVP_MD *md = EVP_get_digestbynid(phtype);
924-
BIO *bhash;
924+
BIO *bhash = NULL;
925925
uint32_t filebound;
926926
size_t pphlen_sz, sections_factor;
927927

928+
if (rphlen == NULL || ctx == NULL || ctx->options == NULL || ctx->pe_ctx == NULL
929+
|| ctx->options->indata == NULL)
930+
return NULL;
931+
932+
if (md == NULL)
933+
return NULL;
934+
935+
mdlen = EVP_MD_size(md);
936+
if (mdlen <= 0)
937+
return NULL;
938+
928939
/* NumberOfSections indicates the size of the section table,
929940
* which immediately follows the headers, can be up to 65535 under Vista and later */
930941
nsections = GET_UINT16_LE(ctx->options->indata + ctx->pe_ctx->header_size + 6);
@@ -971,13 +982,11 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype)
971982
pagesize, hdrsize);
972983
return NULL; /* FAILED */
973984
}
974-
pphlen = 4 + EVP_MD_size(md);
985+
pphlen = 4 + mdlen;
975986

976-
/* Use size_t arithmetic and check for overflow */
987+
/* Compute an upper bound for result size and guard overflow */
977988
pphlen_sz = (size_t)pphlen;
978989
sections_factor = 3 + (size_t)nsections + ((size_t)ctx->pe_ctx->fileend / pagesize);
979-
980-
/* Check for multiplication overflow */
981990
if (sections_factor > SIZE_MAX / pphlen_sz) {
982991
fprintf(stderr, "Page hash allocation size would overflow\n");
983992
return NULL; /* FAILED */
@@ -989,7 +998,24 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype)
989998
return NULL; /* FAILED */
990999
}
9911000

1001+
/* Determine the file boundary for section data validation */
1002+
filebound = ctx->pe_ctx->sigpos ? ctx->pe_ctx->sigpos : ctx->pe_ctx->fileend;
1003+
1004+
/* Validate section table bounds before reading section headers */
1005+
sect_off = (size_t)ctx->pe_ctx->header_size + 24u + (size_t)opthdr_size;
1006+
sect_tbl = (size_t)nsections * 40u;
1007+
1008+
if (sect_off > (size_t)filebound || sect_tbl > (size_t)filebound - sect_off) {
1009+
fprintf(stderr, "Section table out of bounds: off=%zu size=%zu filebound=%u\n",
1010+
sect_off, sect_tbl, filebound);
1011+
return NULL; /* FAILED */
1012+
}
1013+
sections = (char *)ctx->options->indata + sect_off;
1014+
9921015
bhash = BIO_new(BIO_f_md());
1016+
if (bhash == NULL)
1017+
return NULL;
1018+
9931019
#if defined(__GNUC__)
9941020
#pragma GCC diagnostic push
9951021
#pragma GCC diagnostic ignored "-Wcast-qual"
@@ -1002,7 +1028,10 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype)
10021028
#if defined(__GNUC__)
10031029
#pragma GCC diagnostic pop
10041030
#endif
1005-
BIO_push(bhash, BIO_new(BIO_s_null()));
1031+
if (BIO_push(bhash, BIO_new(BIO_s_null())) == NULL) {
1032+
BIO_free_all(bhash);
1033+
return NULL;
1034+
}
10061035
if (!BIO_write_ex(bhash, ctx->options->indata, ctx->pe_ctx->header_size + 88, &written)
10071036
|| written != ctx->pe_ctx->header_size + 88) {
10081037
BIO_free_all(bhash);
@@ -1014,32 +1043,52 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype)
10141043
BIO_free_all(bhash);
10151044
return NULL; /* FAILED */
10161045
}
1017-
if (!BIO_write_ex(bhash,
1018-
ctx->options->indata + ctx->pe_ctx->header_size + 160 + ctx->pe_ctx->pe32plus*16,
1019-
hdrsize - (ctx->pe_ctx->header_size + 160 + ctx->pe_ctx->pe32plus*16), &written)
1020-
|| written != hdrsize - (ctx->pe_ctx->header_size + 160 + ctx->pe_ctx->pe32plus*16)) {
1046+
off = ctx->pe_ctx->header_size + 160 + (size_t)ctx->pe_ctx->pe32plus * 16;
1047+
if (hdrsize < off) {
1048+
BIO_free_all(bhash);
1049+
return NULL; /* FAILED: header too small */
1050+
}
1051+
if (!BIO_write_ex(bhash, ctx->options->indata + off, (size_t)hdrsize - off, &written)
1052+
|| written != hdrsize - off) {
10211053
BIO_free_all(bhash);
10221054
return NULL; /* FAILED */
10231055
}
1056+
if (pagesize < hdrsize) {
1057+
BIO_free_all(bhash);
1058+
return NULL; /* FAILED: header larger than page */
1059+
}
10241060
zeroes = OPENSSL_zalloc((size_t)pagesize);
1025-
if (!BIO_write_ex(bhash, zeroes, pagesize - hdrsize, &written)
1026-
|| written != pagesize - hdrsize) {
1061+
if (zeroes == NULL) {
1062+
BIO_free_all(bhash);
1063+
return NULL; /* FAILED */
1064+
}
1065+
if (!BIO_write_ex(bhash, zeroes, (size_t)pagesize - (size_t)hdrsize, &written)
1066+
|| written != (size_t)pagesize - (size_t)hdrsize) {
10271067
BIO_free_all(bhash);
10281068
OPENSSL_free(zeroes);
10291069
return NULL; /* FAILED */
10301070
}
10311071
res = OPENSSL_malloc((size_t)phlen);
1072+
if (res == NULL) {
1073+
BIO_free_all(bhash);
1074+
OPENSSL_free(zeroes);
1075+
return NULL; /* FAILED */
1076+
}
10321077
memset(res, 0, 4);
1033-
BIO_gets(bhash, (char*)res + 4, EVP_MD_size(md));
1078+
if (BIO_gets(bhash, (char *)res + 4, mdlen) != mdlen) {
1079+
BIO_free_all(bhash);
1080+
OPENSSL_free(zeroes);
1081+
OPENSSL_free(res);
1082+
return NULL; /* FAILED */
1083+
}
10341084
BIO_free_all(bhash);
1035-
sections = ctx->options->indata + ctx->pe_ctx->header_size + 24 + opthdr_size;
1036-
/* Determine the file boundary for section data validation */
1037-
filebound = ctx->pe_ctx->sigpos ? ctx->pe_ctx->sigpos : ctx->pe_ctx->fileend;
1038-
for (i=0; i<nsections; i++) {
1085+
bhash = NULL;
1086+
1087+
for (i = 0; i < (int)nsections; i++) {
10391088
/* SizeOfRawData and PointerToRawData from section header */
10401089
rs = GET_UINT32_LE(sections + 16);
10411090
ro = GET_UINT32_LE(sections + 20);
1042-
if (rs == 0 || rs >= UINT32_MAX) {
1091+
if (rs == 0) {
10431092
sections += 40;
10441093
continue;
10451094
}
@@ -1051,9 +1100,27 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype)
10511100
OPENSSL_free(res);
10521101
return NULL; /* FAILED */
10531102
}
1054-
for (l=0; l<rs; l+=pagesize, pi++) {
1055-
PUT_UINT32_LE(ro + l, res + pi*pphlen);
1103+
for (l = 0; l < rs; l += pagesize, pi++) {
1104+
need = (size_t)(pi + 1) * (size_t)pphlen;
1105+
1106+
/* Prevent OOB write into res if pi grows beyond allocated factor */
1107+
if (need > (size_t)phlen) {
1108+
fprintf(stderr, "Page hash buffer overflow prevented: pi=%d need=%zu phlen=%d\n",
1109+
pi, need, phlen);
1110+
OPENSSL_free(zeroes);
1111+
OPENSSL_free(res);
1112+
return NULL; /* FAILED */
1113+
}
1114+
1115+
PUT_UINT32_LE(ro + l, res + (size_t)pi * (size_t)pphlen);
1116+
10561117
bhash = BIO_new(BIO_f_md());
1118+
if (bhash == NULL) {
1119+
OPENSSL_free(zeroes);
1120+
OPENSSL_free(res);
1121+
return NULL;
1122+
}
1123+
10571124
#if defined(__GNUC__)
10581125
#pragma GCC diagnostic push
10591126
#pragma GCC diagnostic ignored "-Wcast-qual"
@@ -1068,17 +1135,24 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype)
10681135
#if defined(__GNUC__)
10691136
#pragma GCC diagnostic pop
10701137
#endif
1071-
BIO_push(bhash, BIO_new(BIO_s_null()));
1072-
if (rs - l < pagesize) {
1073-
if (!BIO_write_ex(bhash, ctx->options->indata + ro + l, rs - l, &written)
1074-
|| written != rs - l) {
1138+
if (BIO_push(bhash, BIO_new(BIO_s_null())) == NULL) {
1139+
BIO_free_all(bhash);
1140+
OPENSSL_free(zeroes);
1141+
OPENSSL_free(res);
1142+
return NULL;
1143+
}
1144+
if (l < rs && rs - l < pagesize) {
1145+
size_t tail = (size_t)(rs - l);
1146+
1147+
if (!BIO_write_ex(bhash, ctx->options->indata + ro + l, tail, &written)
1148+
|| written != tail) {
10751149
BIO_free_all(bhash);
10761150
OPENSSL_free(zeroes);
10771151
OPENSSL_free(res);
10781152
return NULL; /* FAILED */
10791153
}
1080-
if (!BIO_write_ex(bhash, zeroes, pagesize - (rs - l), &written)
1081-
|| written != pagesize - (rs - l)) {
1154+
if (!BIO_write_ex(bhash, zeroes, pagesize - tail, &written)
1155+
|| written != pagesize - tail) {
10821156
BIO_free_all(bhash);
10831157
OPENSSL_free(zeroes);
10841158
OPENSSL_free(res);
@@ -1093,17 +1167,35 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype)
10931167
return NULL; /* FAILED */
10941168
}
10951169
}
1096-
BIO_gets(bhash, (char*)res + pi*pphlen + 4, EVP_MD_size(md));
1170+
if (BIO_gets(bhash, (char *)res + (size_t)pi * (size_t)pphlen + 4, mdlen) != mdlen) {
1171+
BIO_free_all(bhash);
1172+
OPENSSL_free(zeroes);
1173+
OPENSSL_free(res);
1174+
return NULL; /* FAILED */
1175+
}
10971176
BIO_free_all(bhash);
1177+
bhash = NULL;
10981178
}
10991179
lastpos = ro + rs;
11001180
sections += 40;
11011181
}
1102-
PUT_UINT32_LE(lastpos, res + pi*pphlen);
1103-
memset(res + pi*pphlen + 4, 0, (size_t)EVP_MD_size(md));
1182+
/* Final entry */
1183+
need = (size_t)(pi + 1) * (size_t)pphlen;
1184+
1185+
if (need > (size_t)phlen) {
1186+
fprintf(stderr, "Page hash buffer overflow prevented at final entry: pi=%d need=%zu phlen=%d\n",
1187+
pi, need, phlen);
1188+
OPENSSL_free(zeroes);
1189+
OPENSSL_free(res);
1190+
return NULL; /* FAILED */
1191+
}
1192+
1193+
PUT_UINT32_LE(lastpos, res + (size_t)pi * (size_t)pphlen);
1194+
memset(res + (size_t)pi * (size_t)pphlen + 4, 0, (size_t)mdlen);
11041195
pi++;
1196+
11051197
OPENSSL_free(zeroes);
1106-
*rphlen = pi*pphlen;
1198+
*rphlen = pi * pphlen;
11071199
return res;
11081200
}
11091201

0 commit comments

Comments
 (0)