bryson/pacman
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix CVE-2016-5434 (DoS/loop and out of boundary read)

Browse source 
This is a rewrite of Tobias Stoeckmann’s patch from June 2016[1] using
functions instead of macros. (Thanks to Tobias for explanations of his patch.)
A short question on Freenode IRC showed that macros are generally discouraged
and functions should be used.

The patch introduces a static size_t length_check() in libalpm/signing.c.

[1] Original patch:
https://lists.archlinux.org/pipermail/pacman-dev/2016-June/021148.html
CVE request (and assignment):
http://seclists.org/oss-sec/2016/q2/526

Signed-off-by: Allan McRae <allan@archlinux.org>
This commit is contained in:
Nils Freydank 2017-10-20 22:42:32 +02:00 • committed by Allan McRae
parent 44f3a15798
commit ad0517d371
1 changed files with 44 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

48
lib/libalpm/signing.c
View file

@ -986,6 +986,19 @@ int SYMEXPORT alpm_siglist_cleanup(alpm_siglist_t *siglist)
return 0; return 0;
} }
/* Check to avoid out of boundary reads */
static size_t length_check(size_t length, size_t position, size_t a,
alpm_handle_t *handle, const char *identifier)
{
if( a == 0 || length - position <= a) {
_alpm_log(handle, ALPM_LOG_ERROR,
_("%s: signature format error"), identifier);
return -1;
} else {
return 0;
}
}
/** /**
* Extract the Issuer Key ID from a signature * Extract the Issuer Key ID from a signature
* @param sig PGP signature * @param sig PGP signature
@ -1022,16 +1035,25 @@ int SYMEXPORT alpm_extract_keyid(alpm_handle_t *handle, const char *identifier,
switch(sig[pos] & 0x03) { switch(sig[pos] & 0x03) {
case 0: case 0:
if(length_check(len, pos, 2, handle, identifier) != 0) {
return -1;
}
blen = sig[pos + 1]; blen = sig[pos + 1];
pos = pos + 2; pos = pos + 2;
break; break;
case 1: case 1:
if(length_check(len, pos, 3, handle, identifier)) {
return -1;
}
blen = (sig[pos + 1] << 8) | sig[pos + 2]; blen = (sig[pos + 1] << 8) | sig[pos + 2];
pos = pos + 3; pos = pos + 3;
break; break;
case 2: case 2:
if(length_check(len, pos, 5, handle, identifier)) {
return -1;
}
blen = (sig[pos + 1] << 24) | (sig[pos + 2] << 16) | (sig[pos + 3] << 8) | sig[pos + 4]; blen = (sig[pos + 1] << 24) | (sig[pos + 2] << 16) | (sig[pos + 3] << 8) | sig[pos + 4];
pos = pos + 5; pos = pos + 5;
break; break;
@ -1059,7 +1081,16 @@ int SYMEXPORT alpm_extract_keyid(alpm_handle_t *handle, const char *identifier,
pos = pos + 4; pos = pos + 4;
/* pos got changed above, so an explicit check is necessary
* check for 2 as that catches another some lines down */
if(length_check(len, pos, 2, handle, identifier)) {
return -1;
}
hlen = (sig[pos] << 8) | sig[pos + 1]; hlen = (sig[pos] << 8) | sig[pos + 1];
if(length_check(len, pos, hlen + 2, handle, identifier)) {
return -1;
}
pos = pos + hlen + 2; pos = pos + hlen + 2;
ulen = (sig[pos] << 8) | sig[pos + 1]; ulen = (sig[pos] << 8) | sig[pos + 1];
@ -1072,30 +1103,39 @@ int SYMEXPORT alpm_extract_keyid(alpm_handle_t *handle, const char *identifier,
slen = sig[spos]; slen = sig[spos];
spos = spos + 1; spos = spos + 1;
} else if(sig[spos] < 255) { } else if(sig[spos] < 255) {
if(length_check(pos + ulen, spos, 2, handle, identifier)){
return -1;
}
slen = (sig[spos] << 8) | sig[spos + 1]; slen = (sig[spos] << 8) | sig[spos + 1];
spos = spos + 2; spos = spos + 2;
} else { } else {
/* check for pos and spos, as spos is still pos */
if(length_check(len, pos, 5, handle, identifier)) {
return -1;
}
slen = (sig[spos + 1] << 24) | (sig[spos + 2] << 16) | (sig[spos + 3] << 8) | sig[spos + 4]; slen = (sig[spos + 1] << 24) | (sig[spos + 2] << 16) | (sig[spos + 3] << 8) | sig[spos + 4];
spos = spos + 5; spos = spos + 5;
} }
if(sig[spos] == 16) { if(sig[spos] == 16) {
/* issuer key ID */ /* issuer key ID */
char key[17]; char key[17];
size_t i; size_t i;
if(length_check(pos + ulen, spos, 8, handle, identifier)) {
return -1;
}
for (i = 0; i < 8; i++) { for (i = 0; i < 8; i++) {
sprintf(&key[i * 2], "%02X", sig[spos + i + 1]); sprintf(&key[i * 2], "%02X", sig[spos + i + 1]);
} }
*keys = alpm_list_add(*keys, strdup(key)); *keys = alpm_list_add(*keys, strdup(key));
break; break;
} }
if(length_check(pos + ulen + 1, spos, slen, handle, identifier)) {
return -1;
}
spos = spos + slen; spos = spos + slen;
} }
pos = pos + (blen - hlen - 8); pos = pos + (blen - hlen - 8);
} }
return 0; return 0;
} }