xfrm_user: uncoditionally validate esn replay attribute struct

The sanity test added in 6d91b5f4fd5a1 can be bypassed, validation
only occurs if XFRM_STATE_ESN flag is set, but rest of code doesn't care
and just checks if the attribute itself is present.

So always validate.  Alternative is to reject if we have the attribute
without the flag but that would change abi.

Reported-by: syzbot+0ab777c27d2bb7588f73@syzkaller.appspotmail.com
Cc: Mathias Krause <minipli@googlemail.com>
Fixes: 6d91b5f4fd5a1 ("xfrm_user: ensure user supplied esn replay window is valid")
Fixes: 9e6c072da8f5c ("xfrm: Add user interface for esn and big anti-replay windows")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 7f52b8eb177db4978750caed402aa34f6a31d7b4..080035f056d992c49f8cbcc776d579c9769c67eb 100644 (file)
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -121,22 +121,17 @@ static inline int verify_replay(struct xfrm_usersa_info *p,
        struct nlattr *rt = attrs[XFRMA_REPLAY_ESN_VAL];
        struct xfrm_replay_state_esn *rs;
 
-       if (p->flags & XFRM_STATE_ESN) {
-               if (!rt)
-                       return -EINVAL;
+       if (!rt)
+               return (p->flags & XFRM_STATE_ESN) ? -EINVAL : 0;
 
-               rs = nla_data(rt);
+       rs = nla_data(rt);
 
-               if (rs->bmp_len > XFRMA_REPLAY_ESN_MAX / sizeof(rs->bmp[0]) / 8)
-                       return -EINVAL;
-
-               if (nla_len(rt) < (int)xfrm_replay_state_esn_len(rs) &&
-                   nla_len(rt) != sizeof(*rs))
-                       return -EINVAL;
-       }
+       if (rs->bmp_len > XFRMA_REPLAY_ESN_MAX / sizeof(rs->bmp[0]) / 8)
+               return -EINVAL;
 
-       if (!rt)
-               return 0;
+       if (nla_len(rt) < (int)xfrm_replay_state_esn_len(rs) &&
+           nla_len(rt) != sizeof(*rs))
+               return -EINVAL;
 
        /* As only ESP and AH support ESN feature. */
        if ((p->id.proto != IPPROTO_ESP) && (p->id.proto != IPPROTO_AH))