Patches to fix undefined behavior
David Leon Gil
coruus at gmail.com
Tue Sep 2 15:42:58 CEST 2014
These patches are against 1.4.18.
1&2/3: Correct undefined behavior due to failure to cast type-promoted values to
unsigned int before left-shifting.
This fixes a number of irritating errors when running GPG built with UBSan.
For example, when running './g10/gpg --list-packets checks/plain-1-pgp.asc':
des.c:479:3: runtime error: left shift of 242 by 24 places cannot be
represented in type 'int'
des.c:479:3: runtime error: left shift of 246 by 24 places cannot be
represented in type 'int'
des.c:695:3: runtime error: left shift of 254 by 24 places cannot be
represented in type 'int'
des.c:695:3: runtime error: left shift of 182 by 24 places cannot be
represented in type 'int'
gpg: armor header: Version: GnuPG v1.3.5-cvs (GNU/Linux)
parse-packet.c:100:31: runtime error: left shift of 203 by 24 places
cannot be represented in type 'int'
:pubkey enc packet: version 3, algo 16, keyid 5B7A02F0CB879DE9
data: [765 bits]
data: [766 bits]
gpg: public key is 0x5B7A02F0CB879DE9
keyid.c:303:22: runtime error: left shift of 131 by 24 places cannot
be represented in type 'int'
keyid.c:304:22: runtime error: left shift of 209 by 24 places cannot
be represented in type 'int'
:encrypted data packet:
length: unknown
keyid.c:357:22: runtime error: left shift of 131 by 24 places cannot
be represented in type 'int'
keyid.c:358:22: runtime error: left shift of 209 by 24 places cannot
be represented in type 'int'
gpg: encrypted with ELG-E key, ID 0x5B7A02F0CB879DE9
gpg: decryption failed: secret key not available
3/3: Correct undefined behavior due to dereferencing a null pointer in
util/secmem.c. Note: It is unclear what the code in util/secmem.c that
dereferences a null pointer is intended to calculate. All tests pass
with the replacement code here, but...
- David
>From e8e3f460b5ecf88eda4546cf1b74a7fb1f4c6038 Mon Sep 17 00:00:00 2001
From: David Leon Gil <coruus at gmail.com>
Date: Tue, 2 Sep 2014 09:29:20 -0400
Subject: [PATCH 1/3] Fix undefined behavior in g10/ and util/ due to failure
to properly cast type-promoted integers.
This patch was generated by applying the following Cocci spatch; all
patch-points have been manually checked for correctness.
@@
expression x;
@@
- x << 24
+ (unsigned int)(x) << 24
---
g10/apdu.c | 20 ++++++++++----------
g10/app-openpgp.c | 2 +-
g10/build-packet.c | 2 +-
g10/ccid-driver.c | 2 +-
g10/keygen.c | 4 ++--
g10/keyid.c | 20 ++++++++++----------
g10/misc.c | 2 +-
g10/parse-packet.c | 8 ++++----
util/iobuf.c | 2 +-
util/simple-gettext.c | 2 +-
10 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/g10/apdu.c b/g10/apdu.c
index 66cf30b..b154235 100644
--- a/g10/apdu.c
+++ b/g10/apdu.c
@@ -916,14 +916,14 @@ pcsc_get_status_wrapped (int slot, unsigned int *status)
i? strerror (errno) : "premature EOF");
goto command_failed;
}
- len = (msgbuf[1] << 24) | (msgbuf[2] << 16) | (msgbuf[3] << 8 ) | msgbuf[4];
+ len = ((unsigned int)(msgbuf[1]) << 24) | (msgbuf[2] << 16) |
(msgbuf[3] << 8 ) | msgbuf[4];
if (msgbuf[0] != 0x81 || len < 4)
{
log_error ("invalid response header from PC/SC received\n");
goto command_failed;
}
len -= 4; /* Already read the error code. */
- err = PCSC_ERR_MASK ((msgbuf[5] << 24) | (msgbuf[6] << 16)
+ err = PCSC_ERR_MASK (((unsigned int)(msgbuf[5]) << 24) | (msgbuf[6] << 16)
| (msgbuf[7] << 8 ) | msgbuf[8]);
if (err)
{
@@ -1084,14 +1084,14 @@ pcsc_send_apdu_wrapped (int slot, unsigned
char *apdu, size_t apdulen,
i? strerror (errno) : "premature EOF");
goto command_failed;
}
- len = (msgbuf[1] << 24) | (msgbuf[2] << 16) | (msgbuf[3] << 8 ) | msgbuf[4];
+ len = ((unsigned int)(msgbuf[1]) << 24) | (msgbuf[2] << 16) |
(msgbuf[3] << 8 ) | msgbuf[4];
if (msgbuf[0] != 0x81 || len < 4)
{
log_error ("invalid response header from PC/SC received\n");
goto command_failed;
}
len -= 4; /* Already read the error code. */
- err = PCSC_ERR_MASK ((msgbuf[5] << 24) | (msgbuf[6] << 16)
+ err = PCSC_ERR_MASK (((unsigned int)(msgbuf[5]) << 24) | (msgbuf[6] << 16)
| (msgbuf[7] << 8 ) | msgbuf[8]);
if (err)
{
@@ -1217,14 +1217,14 @@ close_pcsc_reader_wrapped (int slot)
i? strerror (errno) : "premature EOF");
goto command_failed;
}
- len = (msgbuf[1] << 24) | (msgbuf[2] << 16) | (msgbuf[3] << 8 ) | msgbuf[4];
+ len = ((unsigned int)(msgbuf[1]) << 24) | (msgbuf[2] << 16) |
(msgbuf[3] << 8 ) | msgbuf[4];
if (msgbuf[0] != 0x81 || len < 4)
{
log_error ("invalid response header from PC/SC received\n");
goto command_failed;
}
len -= 4; /* Already read the error code. */
- err = PCSC_ERR_MASK ((msgbuf[5] << 24) | (msgbuf[6] << 16)
+ err = PCSC_ERR_MASK (((unsigned int)(msgbuf[5]) << 24) | (msgbuf[6] << 16)
| (msgbuf[7] << 8 ) | msgbuf[8]);
if (err)
log_error ("pcsc_close failed: %s (0x%lx)\n",
@@ -1405,7 +1405,7 @@ reset_pcsc_reader_wrapped (int slot)
i? strerror (errno) : "premature EOF");
goto command_failed;
}
- len = (msgbuf[1] << 24) | (msgbuf[2] << 16) | (msgbuf[3] << 8 ) | msgbuf[4];
+ len = ((unsigned int)(msgbuf[1]) << 24) | (msgbuf[2] << 16) |
(msgbuf[3] << 8 ) | msgbuf[4];
if (msgbuf[0] != 0x81 || len < 4)
{
log_error ("invalid response header from PC/SC received\n");
@@ -1419,7 +1419,7 @@ reset_pcsc_reader_wrapped (int slot)
sw = SW_HOST_GENERAL_ERROR;
goto command_failed;
}
- err = PCSC_ERR_MASK ((msgbuf[5] << 24) | (msgbuf[6] << 16)
+ err = PCSC_ERR_MASK (((unsigned int)(msgbuf[5]) << 24) | (msgbuf[6] << 16)
| (msgbuf[7] << 8 ) | msgbuf[8]);
if (err)
{
@@ -1719,7 +1719,7 @@ open_pcsc_reader_wrapped (const char *portstr)
i? strerror (errno) : "premature EOF");
goto command_failed;
}
- len = (msgbuf[1] << 24) | (msgbuf[2] << 16) | (msgbuf[3] << 8 ) | msgbuf[4];
+ len = ((unsigned int)(msgbuf[1]) << 24) | (msgbuf[2] << 16) |
(msgbuf[3] << 8 ) | msgbuf[4];
if (msgbuf[0] != 0x81 || len < 4)
{
log_error ("invalid response header from PC/SC received\n");
@@ -1732,7 +1732,7 @@ open_pcsc_reader_wrapped (const char *portstr)
(unsigned long)len);
goto command_failed;
}
- err = PCSC_ERR_MASK ((msgbuf[5] << 24) | (msgbuf[6] << 16)
+ err = PCSC_ERR_MASK (((unsigned int)(msgbuf[5]) << 24) | (msgbuf[6] << 16)
| (msgbuf[7] << 8 ) | msgbuf[8]);
if (err)
{
diff --git a/g10/app-openpgp.c b/g10/app-openpgp.c
index a3a977b..5e03f8d 100644
--- a/g10/app-openpgp.c
+++ b/g10/app-openpgp.c
@@ -744,7 +744,7 @@ send_fprtime_if_not_null (ctrl_t ctrl, const char *keyword,
char numbuf1[50], numbuf2[50];
unsigned long value;
- value = (stamp[0] << 24) | (stamp[1]<<16) | (stamp[2]<<8) | stamp[3];
+ value = ((unsigned int)(stamp[0]) << 24) | (stamp[1]<<16) |
(stamp[2]<<8) | stamp[3];
if (!value)
return;
sprintf (numbuf1, "%d", number);
diff --git a/g10/build-packet.c b/g10/build-packet.c
index abe0181..9b93ff6 100644
--- a/g10/build-packet.c
+++ b/g10/build-packet.c
@@ -585,7 +585,7 @@ delete_sig_subpkt (subpktarea_t *area,
sigsubpkttype_t reqtype )
if( n == 255 ) {
if( buflen < 4 )
break;
- n = (buffer[0] << 24) | (buffer[1] << 16)
+ n = ((unsigned int)(buffer[0]) << 24) | (buffer[1] << 16)
| (buffer[2] << 8) | buffer[3];
buffer += 4;
buflen -= 4;
diff --git a/g10/ccid-driver.c b/g10/ccid-driver.c
index 8c362d7..07037c7 100644
--- a/g10/ccid-driver.c
+++ b/g10/ccid-driver.c
@@ -292,7 +292,7 @@ static int abort_cmd (ccid_driver_t handle, int seqno);
static unsigned int
convert_le_u32 (const unsigned char *buf)
{
- return buf[0] | (buf[1] << 8) | (buf[2] << 16) | (buf[3] << 24);
+ return buf[0] | (buf[1] << 8) | (buf[2] << 16) | ((unsigned
int)(buf[3]) << 24);
}
diff --git a/g10/keygen.c b/g10/keygen.c
index 84f852f..23d80a3 100644
--- a/g10/keygen.c
+++ b/g10/keygen.c
@@ -832,7 +832,7 @@ make_backsig (PKT_signature *sig, PKT_public_key *pk,
}
else if(buf[1]==255)
{
- pktlen =buf[2] << 24;
+ pktlen =(unsigned int)(buf[2]) << 24;
pktlen|=buf[3] << 16;
pktlen|=buf[4] << 8;
pktlen|=buf[5];
@@ -852,7 +852,7 @@ make_backsig (PKT_signature *sig, PKT_public_key *pk,
break;
case 2:
- pktlen =buf[mark++] << 24;
+ pktlen =(unsigned int)(buf[mark++]) << 24;
pktlen|=buf[mark++] << 16;
case 1:
diff --git a/g10/keyid.c b/g10/keyid.c
index d7072d4..cf0725f 100644
--- a/g10/keyid.c
+++ b/g10/keyid.c
@@ -241,11 +241,11 @@ keystr_from_desc(KEYDB_SEARCH_DESC *desc)
{
u32 keyid[2];
- keyid[0] = (unsigned char)desc->u.fpr[12] << 24
+ keyid[0] = (unsigned int)((unsigned char)desc->u.fpr[12]) << 24
| (unsigned char)desc->u.fpr[13] << 16
| (unsigned char)desc->u.fpr[14] << 8
| (unsigned char)desc->u.fpr[15] ;
- keyid[1] = (unsigned char)desc->u.fpr[16] << 24
+ keyid[1] = (unsigned int)((unsigned char)desc->u.fpr[16]) << 24
| (unsigned char)desc->u.fpr[17] << 16
| (unsigned char)desc->u.fpr[18] << 8
| (unsigned char)desc->u.fpr[19] ;
@@ -300,8 +300,8 @@ keyid_from_sk( PKT_secret_key *sk, u32 *keyid )
if(md)
{
dp = md_read( md, 0 );
- keyid[0] = dp[12] << 24 | dp[13] << 16 | dp[14] << 8 | dp[15] ;
- keyid[1] = dp[16] << 24 | dp[17] << 16 | dp[18] << 8 | dp[19] ;
+ keyid[0] = (unsigned int)(dp[12]) << 24 | dp[13] << 16 | dp[14] <<
8 | dp[15] ;
+ keyid[1] = (unsigned int)(dp[16]) << 24 | dp[17] << 16 | dp[18] <<
8 | dp[19] ;
lowbits = keyid[1];
md_close(md);
sk->keyid[0] = keyid[0];
@@ -354,8 +354,8 @@ keyid_from_pk( PKT_public_key *pk, u32 *keyid )
if(md)
{
dp = md_read( md, 0 );
- keyid[0] = dp[12] << 24 | dp[13] << 16 | dp[14] << 8 | dp[15] ;
- keyid[1] = dp[16] << 24 | dp[17] << 16 | dp[18] << 8 | dp[19] ;
+ keyid[0] = (unsigned int)(dp[12]) << 24 | dp[13] << 16 | dp[14] <<
8 | dp[15] ;
+ keyid[1] = (unsigned int)(dp[16]) << 24 | dp[17] << 16 | dp[18] <<
8 | dp[19] ;
lowbits = keyid[1];
md_close(md);
pk->keyid[0] = keyid[0];
@@ -398,8 +398,8 @@ keyid_from_fingerprint( const byte *fprint, size_t
fprint_len, u32 *keyid )
}
else {
const byte *dp = fprint;
- keyid[0] = dp[12] << 24 | dp[13] << 16 | dp[14] << 8 | dp[15] ;
- keyid[1] = dp[16] << 24 | dp[17] << 16 | dp[18] << 8 | dp[19] ;
+ keyid[0] = (unsigned int)(dp[12]) << 24 | dp[13] << 16 | dp[14] << 8
| dp[15] ;
+ keyid[1] = (unsigned int)(dp[16]) << 24 | dp[17] << 16 | dp[18] << 8
| dp[19] ;
}
return keyid[1];
@@ -687,8 +687,8 @@ fingerprint_from_pk( PKT_public_key *pk, byte
*array, size_t *ret_len )
if( !array )
array = xmalloc( len );
memcpy(array, dp, len );
- pk->keyid[0] = dp[12] << 24 | dp[13] << 16 | dp[14] << 8 | dp[15] ;
- pk->keyid[1] = dp[16] << 24 | dp[17] << 16 | dp[18] << 8 | dp[19] ;
+ pk->keyid[0] = (unsigned int)(dp[12]) << 24 | dp[13] << 16 | dp[14]
<< 8 | dp[15] ;
+ pk->keyid[1] = (unsigned int)(dp[16]) << 24 | dp[17] << 16 | dp[18]
<< 8 | dp[19] ;
md_close(md);
}
diff --git a/g10/misc.c b/g10/misc.c
index 68b4cea..e443836 100644
--- a/g10/misc.c
+++ b/g10/misc.c
@@ -299,7 +299,7 @@ u32
buffer_to_u32( const byte *buffer )
{
unsigned long a;
- a = *buffer << 24;
+ a = (unsigned int)(*buffer) << 24;
a |= buffer[1] << 16;
a |= buffer[2] << 8;
a |= buffer[3];
diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index dcda8ef..92fc399 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -97,7 +97,7 @@ static unsigned long
read_32(IOBUF inp)
{
unsigned long a;
- a = iobuf_get_noeof(inp) << 24;
+ a = (unsigned int)(iobuf_get_noeof(inp)) << 24;
a |= iobuf_get_noeof(inp) << 16;
a |= iobuf_get_noeof(inp) << 8;
a |= iobuf_get_noeof(inp);
@@ -377,7 +377,7 @@ parse( IOBUF inp, PACKET *pkt, int onlykeypkts,
off_t *retpos,
}
else if( c == 255 )
{
- pktlen = (hdr[hdrlen++] = iobuf_get_noeof(inp)) << 24;
+ pktlen = (unsigned int)((hdr[hdrlen++] = iobuf_get_noeof(inp))) << 24;
pktlen |= (hdr[hdrlen++] = iobuf_get_noeof(inp)) << 16;
pktlen |= (hdr[hdrlen++] = iobuf_get_noeof(inp)) << 8;
if( (c = iobuf_get(inp)) == -1 )
@@ -1178,7 +1178,7 @@ enum_sig_subpkt( const subpktarea_t *pktbuf,
sigsubpkttype_t reqtype,
if( n == 255 ) { /* 4 byte length header */
if( buflen < 4 )
goto too_short;
- n = (buffer[0] << 24) | (buffer[1] << 16)
+ n = ((unsigned int)(buffer[0]) << 24) | (buffer[1] << 16)
| (buffer[2] << 8) | buffer[3];
buffer += 4;
buflen -= 4;
@@ -2011,7 +2011,7 @@ parse_attribute_subpkts(PKT_user_id *uid)
if( n == 255 ) { /* 4 byte length header */
if( buflen < 4 )
goto too_short;
- n = (buffer[0] << 24) | (buffer[1] << 16)
+ n = ((unsigned int)(buffer[0]) << 24) | (buffer[1] << 16)
| (buffer[2] << 8) | buffer[3];
buffer += 4;
buflen -= 4;
diff --git a/util/iobuf.c b/util/iobuf.c
index 35de020..ec931d5 100644
--- a/util/iobuf.c
+++ b/util/iobuf.c
@@ -738,7 +738,7 @@ block_filter(void *opaque, int control, IOBUF
chain, byte *buf, size_t *ret_len)
}
}
else if( c == 255 ) {
- a->size = iobuf_get(chain) << 24;
+ a->size = (unsigned int)(iobuf_get(chain)) << 24;
a->size |= iobuf_get(chain) << 16;
a->size |= iobuf_get(chain) << 8;
if( (c = iobuf_get(chain)) == -1 ) {
diff --git a/util/simple-gettext.c b/util/simple-gettext.c
index 9225737..7751efb 100644
--- a/util/simple-gettext.c
+++ b/util/simple-gettext.c
@@ -107,7 +107,7 @@ static struct loaded_domain *the_domain;
static __inline__ u32
do_swap_u32( u32 i )
{
- return (i << 24) | ((i & 0xff00) << 8) | ((i >> 8) & 0xff00) | (i >> 24);
+ return ((unsigned int)(i) << 24) | ((i & 0xff00) << 8) | ((i >> 8)
& 0xff00) | (i >> 24);
}
#define SWAPIT(flag, data) ((flag) ? do_swap_u32(data) : (data) )
--
1.8.5.2 (Apple Git-48)
>From 77512ffe117e951e49fa6a4b67aa85c82afda960 Mon Sep 17 00:00:00 2001
From: David Leon Gil <coruus at gmail.com>
Date: Tue, 2 Sep 2014 09:30:29 -0400
Subject: [PATCH 2/3] Fix undefined behavior in cipher/ due to failure to
properly cast type-promoted integers.
(This includes an instance of the idiom which Coccinelle failed to
produce a correct patch for.)
---
cipher/blowfish.c | 8 ++++----
cipher/cast5.c | 16 ++++++++--------
cipher/des.c | 4 ++--
cipher/twofish.c | 2 +-
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/cipher/blowfish.c b/cipher/blowfish.c
index 61cd2b7..3fb0892 100644
--- a/cipher/blowfish.c
+++ b/cipher/blowfish.c
@@ -424,8 +424,8 @@ do_encrypt_block( BLOWFISH_context *bc, byte
*outbuf, const byte *inbuf )
{
u32 d1, d2;
- d1 = inbuf[0] << 24 | inbuf[1] << 16 | inbuf[2] << 8 | inbuf[3];
- d2 = inbuf[4] << 24 | inbuf[5] << 16 | inbuf[6] << 8 | inbuf[7];
+ d1 = (unsigned int)(inbuf[0]) << 24 | inbuf[1] << 16 | inbuf[2]
<< 8 | inbuf[3];
+ d2 = (unsigned int)(inbuf[4]) << 24 | inbuf[5] << 16 | inbuf[6]
<< 8 | inbuf[7];
do_encrypt( bc, &d1, &d2 );
outbuf[0] = (d1 >> 24) & 0xff;
outbuf[1] = (d1 >> 16) & 0xff;
@@ -449,8 +449,8 @@ do_decrypt_block( BLOWFISH_context *bc, byte
*outbuf, const byte *inbuf )
{
u32 d1, d2;
- d1 = inbuf[0] << 24 | inbuf[1] << 16 | inbuf[2] << 8 | inbuf[3];
- d2 = inbuf[4] << 24 | inbuf[5] << 16 | inbuf[6] << 8 | inbuf[7];
+ d1 = (unsigned int)(inbuf[0]) << 24 | inbuf[1] << 16 | inbuf[2]
<< 8 | inbuf[3];
+ d2 = (unsigned int)(inbuf[4]) << 24 | inbuf[5] << 16 | inbuf[6]
<< 8 | inbuf[7];
decrypt( bc, &d1, &d2 );
outbuf[0] = (d1 >> 24) & 0xff;
outbuf[1] = (d1 >> 16) & 0xff;
diff --git a/cipher/cast5.c b/cipher/cast5.c
index ed8c738..c3abc67 100644
--- a/cipher/cast5.c
+++ b/cipher/cast5.c
@@ -375,8 +375,8 @@ do_encrypt_block( CAST5_context *c, byte *outbuf,
const byte *inbuf )
/* (L0,R0) <-- (m1...m64). (Split the plaintext into left and
* right 32-bit halves L0 = m1...m32 and R0 = m33...m64.)
*/
- l = inbuf[0] << 24 | inbuf[1] << 16 | inbuf[2] << 8 | inbuf[3];
- r = inbuf[4] << 24 | inbuf[5] << 16 | inbuf[6] << 8 | inbuf[7];
+ l = (unsigned int)(inbuf[0]) << 24 | inbuf[1] << 16 | inbuf[2] <<
8 | inbuf[3];
+ r = (unsigned int)(inbuf[4]) << 24 | inbuf[5] << 16 | inbuf[6] <<
8 | inbuf[7];
/* (16 rounds) for i from 1 to 16, compute Li and Ri as follows:
* Li = Ri-1;
@@ -433,8 +433,8 @@ do_decrypt_block (CAST5_context *c, byte *outbuf,
const byte *inbuf )
Km = c->Km;
Kr = c->Kr;
- l = inbuf[0] << 24 | inbuf[1] << 16 | inbuf[2] << 8 | inbuf[3];
- r = inbuf[4] << 24 | inbuf[5] << 16 | inbuf[6] << 8 | inbuf[7];
+ l = (unsigned int)(inbuf[0]) << 24 | inbuf[1] << 16 | inbuf[2] <<
8 | inbuf[3];
+ r = (unsigned int)(inbuf[4]) << 24 | inbuf[5] << 16 | inbuf[6] <<
8 | inbuf[7];
t = l; l = r; r = t ^ F1(r, Km[15], Kr[15]);
t = l; l = r; r = t ^ F3(r, Km[14], Kr[14]);
@@ -588,10 +588,10 @@ do_cast_setkey( CAST5_context *c, const byte
*key, unsigned keylen )
if( keylen != 16 )
return G10ERR_WRONG_KEYLEN;
- x[0] = key[0] << 24 | key[1] << 16 | key[2] << 8 | key[3];
- x[1] = key[4] << 24 | key[5] << 16 | key[6] << 8 | key[7];
- x[2] = key[8] << 24 | key[9] << 16 | key[10] << 8 | key[11];
- x[3] = key[12] << 24 | key[13] << 16 | key[14] << 8 | key[15];
+ x[0] = (unsigned int)(key[0]) << 24 | key[1] << 16 | key[2] <<
8 | key[3];
+ x[1] = (unsigned int)(key[4]) << 24 | key[5] << 16 | key[6] <<
8 | key[7];
+ x[2] = (unsigned int)(key[8]) << 24 | key[9] << 16 | key[10] <<
8 | key[11];
+ x[3] = (unsigned int)(key[12]) << 24 | key[13] << 16 | key[14] <<
8 | key[15];
key_schedule( x, z, k );
for(i=0; i < 16; i++ )
diff --git a/cipher/des.c b/cipher/des.c
index 756c146..f5934e5 100644
--- a/cipher/des.c
+++ b/cipher/des.c
@@ -430,8 +430,8 @@ static byte weak_keys[64][8] =
* Macros to convert 8 bytes from/to 32bit words.
*/
#define READ_64BIT_DATA(data, left, right) \
- left = (data[0] << 24) | (data[1] << 16) | (data[2] << 8) | data[3]; \
- right = (data[4] << 24) | (data[5] << 16) | (data[6] << 8) | data[7];
+ left = ((unsigned int)data[0] << 24) | (data[1] << 16) |
(data[2] << 8) | data[3]; \
+ right = ((unsigned int)data[4] << 24) | (data[5] << 16) |
(data[6] << 8) | data[7];
#define WRITE_64BIT_DATA(data, left, right) \
data[0] = (left >> 24) &0xff; data[1] = (left >> 16) &0xff; \
diff --git a/cipher/twofish.c b/cipher/twofish.c
index 2feccdf..603a806 100644
--- a/cipher/twofish.c
+++ b/cipher/twofish.c
@@ -756,7 +756,7 @@ twofish_setkey (void *ctx, const byte *key,
unsigned int keylen)
#define INPACK(n, x, m) \
x = in[4 * (n)] ^ (in[4 * (n) + 1] << 8) \
- ^ (in[4 * (n) + 2] << 16) ^ (in[4 * (n) + 3] << 24) ^ ctx->w[m]
+ ^ (in[4 * (n) + 2] << 16) ^ ((unsigned int)(in[4 * (n) + 3]) <<
24) ^ ctx->w[m]
#define OUTUNPACK(n, x, m) \
x ^= ctx->w[m]; \
--
1.8.5.2 (Apple Git-48)
>From 34adb565128a8b40aee1a69037e9c598e9086346 Mon Sep 17 00:00:00 2001
From: David Leon Gil <coruus at gmail.com>
Date: Tue, 2 Sep 2014 09:32:48 -0400
Subject: [PATCH 3/3] Fix undefined behavior due to dereferencing a NULL
pointer in util/secmem.c.
Adds a static MEMBLOCK, which is used to calculate the same(?) offset
that the previous code attempted to calculate.
---
util/secmem.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/util/secmem.c b/util/secmem.c
index 553175b..3123e5b 100644
--- a/util/secmem.c
+++ b/util/secmem.c
@@ -68,6 +68,7 @@ struct memblock_struct {
} u;
};
+static const MEMBLOCK mbforalignment;
static void *pool;
@@ -415,7 +416,8 @@ secmexrealloc( void *p, size_t newsize )
size_t size;
void *a;
- mb = (MEMBLOCK*)((char*)p - ((size_t) &((MEMBLOCK*)0)->u.aligned.c));
+ mb = (MEMBLOCK*)((char*)p -
((size_t)((uintptr_t)(&(mbforalignment.u.aligned.c)) -
(uintptr_t)(&mbforalignment)))
+);
size = mb->size;
if (size < sizeof(MEMBLOCK))
log_bug ("secure memory corrupted at block %p\n", (void *)mb);
@@ -442,7 +444,8 @@ secmem_free( void *a )
if( !a )
return;
- mb = (MEMBLOCK*)((char*)a - ((size_t) &((MEMBLOCK*)0)->u.aligned.c));
+ mb = (MEMBLOCK*)((char*)a -
((size_t)((uintptr_t)(&(mbforalignment.u.aligned.c)) -
(uintptr_t)(&mbforalignment)))
+);
size = mb->size;
/* This does not make much sense: probably this memory is held in the
* cache. We do it anyway: */
--
1.8.5.2 (Apple Git-48)
More information about the Gnupg-devel
mailing list