[pacman-dev] [Patch 1/1] Fix CVE-2016-5434 (DoS/loop and out of boundary read)
Nils Freydank
holgersson at posteo.de
Tue Sep 5 16:04:11 UTC 2017
This is an update to fix style issues (indentation, newlines etc.) that were addressed on IRC.
Original message:
>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 size_t length_check() and applies it to
>libalpm/signing.c and signing.h.
>
>Feedback is very appreciated.
>
>
>[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
diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
index 95cb3280..3a907bb8 100644
--- a/lib/libalpm/signing.c
+++ b/lib/libalpm/signing.c
@@ -986,6 +986,19 @@ int SYMEXPORT alpm_siglist_cleanup(alpm_siglist_t *siglist)
return 0;
}
+/* Check to avoid out of boundary reads */
+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
* @param sig PGP signature
@@ -1022,18 +1035,41 @@ int SYMEXPORT alpm_extract_keyid(alpm_handle_t *handle, const char *identifier,
switch(sig[pos] & 0x03) {
case 0:
- blen = sig[pos + 1];
- pos = pos + 2;
+ if(length_check(len, pos, 1, handle, identifier)) {
+ return -1;
+ } else {
+ blen = sig[pos + 1];
+ }
+ if(length_check(len, pos, 2, handle, identifier)) {
+ return -1;
+ } else {
+ pos = pos + 2;
+ }
break;
-
case 1:
- blen = (sig[pos + 1] << 8) | sig[pos + 2];
- pos = pos + 3;
+ if(length_check(len, pos, 2, handle, identifier)) {
+ return -1;
+ } else {
+ blen = (sig[pos + 1] << 8) | sig[pos + 2];
+ }
+ if(length_check(len, pos, 3, handle, identifier)) {
+ return -1;
+ } else {
+ pos = pos + 3;
+ }
break;
case 2:
- blen = (sig[pos + 1] << 24) | (sig[pos + 2] << 16) | (sig[pos + 3] << 8) | sig[pos + 4];
- pos = pos + 5;
+ if(length_check(len, pos, 4, handle, identifier)) {
+ return -1;
+ } else {
+ blen = (sig[pos + 1] << 24) | (sig[pos + 2] << 16) | (sig[pos + 3] << 8) | sig[pos + 4];
+ }
+ if(length_check(len, pos, 5, handle, identifier)) {
+ return -1;
+ } else {
+ pos = pos + 5;
+ }
break;
case 3:
@@ -1057,43 +1093,99 @@ int SYMEXPORT alpm_extract_keyid(alpm_handle_t *handle, const char *identifier,
return -1;
}
- pos = pos + 4;
+ if(length_check(len, pos, 4, handle, identifier)) {
+ return -1;
+ } else {
+ pos = pos + 4;
+ }
- hlen = (sig[pos] << 8) | sig[pos + 1];
- pos = pos + hlen + 2;
+ if(length_check(len, pos, 1, handle, identifier)) {
+ return -1;
+ } else {
+ hlen = (sig[pos] << 8) | sig[pos + 1];
+ }
- ulen = (sig[pos] << 8) | sig[pos + 1];
- pos = pos + 2;
+ if(length_check(len, pos, 2, handle, identifier)) {
+ return -1;
+ } else {
+ pos = pos + hlen + 2;
+ }
- spos = pos;
+ if(length_check(len, pos, 1, handle, identifier)) {
+ return -1;
+ } else {
+ ulen = (sig[pos] << 8) | sig[pos + 1];
+ }
- while(spos < pos + ulen) {
- if(sig[spos] < 192) {
- slen = sig[spos];
- spos = spos + 1;
- } else if(sig[spos] < 255) {
- slen = (sig[spos] << 8) | sig[spos + 1];
- spos = spos + 2;
- } else {
- slen = (sig[spos + 1] << 24) | (sig[spos + 2] << 16) | (sig[spos + 3] << 8) | sig[spos + 4];
- spos = spos + 5;
- }
+ if(length_check(len, pos, 2, handle, identifier)) {
+ return -1;
+ } else {
+ pos = pos + 2;
+ }
- if(sig[spos] == 16) {
- /* issuer key ID */
- char key[17];
- size_t i;
- for (i = 0; i < 8; i++) {
- sprintf(&key[i * 2], "%02X", sig[spos + i + 1]);
+ spos = pos;
+
+ if(length_check(len, pos, ulen, handle, identifier)) {
+ return -1;
+ } else {
+ while(spos < pos + ulen) {
+ if(sig[spos] < 192) {
+ slen = sig[spos];
+ if(length_check(len + ulen, spos, 1, handle, identifier)) {
+ return -1;
+ } else {
+ spos = spos + 1;
+ }
+ } else if(sig[spos] < 255) {
+ if(length_check(pos + ulen, spos, 1, handle, identifier))
+ {
+ return -1;
+ } else {
+ slen = (sig[spos] << 8) | sig[spos + 1];
+ }
+ if(length_check(pos + ulen, spos, 2, handle, identifier)) {
+ return -1;
+ } else {
+ spos = spos + 2;
+ }
+ } else {
+ if(length_check(len, pos, 4, handle, identifier)) {
+ return -1;
+ } else {
+ slen = (sig[spos + 1] << 24) | (sig[spos + 2] << 16) | (sig[spos + 3] << 8) | sig[spos + 4];
+ }
+ if(length_check(len, spos, 5, handle, identifier)) {
+ return -1;
+ } else {
+ spos = spos + 5;
+ }
+ }
+ if(sig[spos] == 16) {
+ /* issuer key ID */
+ char key[17];
+ size_t i;
+ if(length_check(pos + ulen, spos, 8, handle, identifier)) {
+ return -1;
+ } else {
+ for (i = 0; i < 8; i++) {
+ sprintf(&key[i * 2], "%02X", sig[spos + i + 1]);
+ }
+ }
+ *keys = alpm_list_add(*keys, strdup(key));
+ break;
+ }
+ if(length_check(pos + ulen + 1, spos, slen, handle, identifier)) {
+ return -1;
+ } else {
+ spos = spos + slen;
}
- *keys = alpm_list_add(*keys, strdup(key));
- break;
}
-
- spos = spos + slen;
+ if(length_check( blen, hlen, 8, handle, identifier )) {
+ return -1;
+ } else {
+ pos = pos + (blen - hlen - 8);
+ }
}
-
- pos = pos + (blen - hlen - 8);
}
return 0;
diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h
index 3507f584..a67bd900 100644
--- a/lib/libalpm/signing.h
+++ b/lib/libalpm/signing.h
@@ -36,4 +36,6 @@ int _alpm_key_import(alpm_handle_t *handle, const char *fpr);
#endif /* ALPM_SIGNING_H */
+size_t length_check(size_t length, size_t position, size_t a,
+ alpm_handle_t *handle, const char *identifier);
/* vim: set noet: */
--
GPG fingerprint: '766B 8122 1342 6912 3401 492A 8B54 D7A3 FF3C DB17'
Holgersson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.archlinux.org/pipermail/pacman-dev/attachments/20170905/6e73f13c/attachment.asc>
More information about the pacman-dev
mailing list