Skip to content

Commit 01dbc71

Browse files
Kevin EastonZhengShunQian
authored andcommitted
af_key: Always verify length of provided sadb_key
commit 4b66af2 upstream. Key extensions (struct sadb_key) include a user-specified number of key bits. The kernel uses that number to determine how much key data to copy out of the message in pfkey_msg2xfrm_state(). The length of the sadb_key message must be verified to be long enough, even in the case of SADB_X_AALG_NULL. Furthermore, the sadb_key_len value must be long enough to include both the key data and the struct sadb_key itself. Introduce a helper function verify_key_len(), and call it from parse_exthdrs() where other exthdr types are similarly checked for correctness. Signed-off-by: Kevin Easton <[email protected]> Reported-by: syzbot+5022a34ca5a3d49b84223653fab632dfb7b4cf37@syzkaller.appspotmail.com Signed-off-by: Steffen Klassert <[email protected]> Cc: Zubin Mithra <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 7a05d45 commit 01dbc71

File tree

1 file changed

+35
-10
lines changed

1 file changed

+35
-10
lines changed

net/key/af_key.c

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,24 @@ static int verify_address_len(const void *p)
437437
return 0;
438438
}
439439

440+
static inline int sadb_key_len(const struct sadb_key *key)
441+
{
442+
int key_bytes = DIV_ROUND_UP(key->sadb_key_bits, 8);
443+
444+
return DIV_ROUND_UP(sizeof(struct sadb_key) + key_bytes,
445+
sizeof(uint64_t));
446+
}
447+
448+
static int verify_key_len(const void *p)
449+
{
450+
const struct sadb_key *key = p;
451+
452+
if (sadb_key_len(key) > key->sadb_key_len)
453+
return -EINVAL;
454+
455+
return 0;
456+
}
457+
440458
static inline int pfkey_sec_ctx_len(const struct sadb_x_sec_ctx *sec_ctx)
441459
{
442460
return DIV_ROUND_UP(sizeof(struct sadb_x_sec_ctx) +
@@ -533,16 +551,25 @@ static int parse_exthdrs(struct sk_buff *skb, const struct sadb_msg *hdr, void *
533551
return -EINVAL;
534552
if (ext_hdrs[ext_type-1] != NULL)
535553
return -EINVAL;
536-
if (ext_type == SADB_EXT_ADDRESS_SRC ||
537-
ext_type == SADB_EXT_ADDRESS_DST ||
538-
ext_type == SADB_EXT_ADDRESS_PROXY ||
539-
ext_type == SADB_X_EXT_NAT_T_OA) {
554+
switch (ext_type) {
555+
case SADB_EXT_ADDRESS_SRC:
556+
case SADB_EXT_ADDRESS_DST:
557+
case SADB_EXT_ADDRESS_PROXY:
558+
case SADB_X_EXT_NAT_T_OA:
540559
if (verify_address_len(p))
541560
return -EINVAL;
542-
}
543-
if (ext_type == SADB_X_EXT_SEC_CTX) {
561+
break;
562+
case SADB_X_EXT_SEC_CTX:
544563
if (verify_sec_ctx_len(p))
545564
return -EINVAL;
565+
break;
566+
case SADB_EXT_KEY_AUTH:
567+
case SADB_EXT_KEY_ENCRYPT:
568+
if (verify_key_len(p))
569+
return -EINVAL;
570+
break;
571+
default:
572+
break;
546573
}
547574
ext_hdrs[ext_type-1] = (void *) p;
548575
}
@@ -1111,14 +1138,12 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net,
11111138
key = ext_hdrs[SADB_EXT_KEY_AUTH - 1];
11121139
if (key != NULL &&
11131140
sa->sadb_sa_auth != SADB_X_AALG_NULL &&
1114-
((key->sadb_key_bits+7) / 8 == 0 ||
1115-
(key->sadb_key_bits+7) / 8 > key->sadb_key_len * sizeof(uint64_t)))
1141+
key->sadb_key_bits == 0)
11161142
return ERR_PTR(-EINVAL);
11171143
key = ext_hdrs[SADB_EXT_KEY_ENCRYPT-1];
11181144
if (key != NULL &&
11191145
sa->sadb_sa_encrypt != SADB_EALG_NULL &&
1120-
((key->sadb_key_bits+7) / 8 == 0 ||
1121-
(key->sadb_key_bits+7) / 8 > key->sadb_key_len * sizeof(uint64_t)))
1146+
key->sadb_key_bits == 0)
11221147
return ERR_PTR(-EINVAL);
11231148

11241149
x = xfrm_state_alloc(net);

0 commit comments

Comments
 (0)