* [bug report] crypto: dh - Add DH software implementation
@ 2019-10-14 11:38 Dan Carpenter
0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2019-10-14 11:38 UTC (permalink / raw)
To: salvatore.benedetto; +Cc: linux-crypto
Hello Salvatore Benedetto,
The patch 802c7f1c84e4: "crypto: dh - Add DH software implementation"
from Jun 22, 2016, leads to the following static checker warning:
crypto/dh_helper.c:99 crypto_dh_decode_key()
warn: potential overflow
crypto/dh_helper.c
68 int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
69 {
70 const u8 *ptr = buf;
71 struct kpp_secret secret;
72
73 if (unlikely(!buf || len < DH_KPP_SECRET_MIN_SIZE))
74 return -EINVAL;
75
76 ptr = dh_unpack_data(&secret, ptr, sizeof(secret));
77 if (secret.type != CRYPTO_KPP_SECRET_TYPE_DH)
78 return -EINVAL;
79
80 ptr = dh_unpack_data(¶ms->key_size, ptr, sizeof(params->key_size));
81 ptr = dh_unpack_data(¶ms->p_size, ptr, sizeof(params->p_size));
82 ptr = dh_unpack_data(¶ms->q_size, ptr, sizeof(params->q_size));
83 ptr = dh_unpack_data(¶ms->g_size, ptr, sizeof(params->g_size));
84 if (secret.len != crypto_dh_key_len(params))
The largest parameter has to be "params->p_size" but it's a u32 from the
user. So crypto_dh_key_len() can have an integer overflow and wrap back
to "secret.len".
85 return -EINVAL;
86
87 /*
88 * Don't permit the buffer for 'key' or 'g' to be larger than 'p', since
89 * some drivers assume otherwise.
90 */
91 if (params->key_size > params->p_size ||
92 params->g_size > params->p_size || params->q_size > params->p_size)
This ensures that "params->p_size" is the largest.
93 return -EINVAL;
94
95 /* Don't allocate memory. Set pointers to data within
96 * the given buffer
97 */
98 params->key = (void *)ptr;
99 params->p = (void *)(ptr + params->key_size);
100 params->q = (void *)(ptr + params->key_size + params->p_size);
This could wrap.
101 params->g = (void *)(ptr + params->key_size + params->p_size +
102 params->q_size);
103
104 /*
105 * Don't permit 'p' to be 0. It's not a prime number, and it's subject
106 * to corner cases such as 'mod 0' being undefined or
107 * crypto_kpp_maxsize() returning 0.
108 */
109 if (memchr_inv(params->p, 0, params->p_size) == NULL)
It would probably/hopefully lead to an Oops in memchr_inv().
110 return -EINVAL;
111
112 /* It is permissible to not provide Q. */
113 if (params->q_size == 0)
114 params->q = NULL;
115
116 return 0;
117 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2019-10-14 11:38 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 11:38 [bug report] crypto: dh - Add DH software implementation Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).