datapath-windows: Validate Netlink packets' integrity.

Solved access violation when trying to access Netlink message - obtained
with forged IOCTLs.

Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com>
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

2 files changed, 66 insertions, 13 deletions

diff --git a/datapath-windows/ovsext/Netlink/Netlink.c b/datapath-windows/ovsext/Netlink/Netlink.c
index 27dcd4f14..dc1e78c60 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -108,12 +108,18 @@ NlFillNlHdr(PNL_BUFFER nlBuf, UINT16 nlmsgType,
  * ---------------------------------------------------------------------------
  */
 VOID
-NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgError, UINT errorCode)
+NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgError,
+                UINT32 msgErrorLen,
+                UINT errorCode, UINT32 *msgLen)
 {
     NL_BUFFER nlBuffer;
 
     ASSERT(errorCode != NL_ERROR_PENDING);
 
+    if ((msgError == NULL) || (msgErrorLen < sizeof *msgError)) {
+        return;
+    }
+
     NlBufInit(&nlBuffer, (PCHAR)msgError, sizeof *msgError);
     NlFillNlHdr(&nlBuffer, NLMSG_ERROR, 0,
                 msgIn->nlMsg.nlmsgSeq, msgIn->nlMsg.nlmsgPid);
@@ -121,6 +127,10 @@ NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgError, UINT errorCode)
     msgError->errorMsg.error = errorCode;
     msgError->errorMsg.nlMsg = msgIn->nlMsg;
     msgError->nlMsg.nlmsgLen = sizeof(OVS_MESSAGE_ERROR);
+
+    if (NULL != msgLen) {
+        *msgLen = msgError->nlMsg.nlmsgLen;
+    }
 }
 
 /*
@@ -1006,7 +1016,7 @@ NlAttrFind__(const PNL_ATTR attrs, UINT32 size, UINT16 type)
 {
     PNL_ATTR iter = NULL;
     PNL_ATTR ret = NULL;
-    UINT32 left;
+    INT left;
 
     NL_ATTR_FOR_EACH (iter, left, attrs, size) {
         if (NlAttrType(iter) == type) {
@@ -1036,6 +1046,49 @@ NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
 
 /*
  *----------------------------------------------------------------------------
+ * Traverses all attributes in received buffer in order to insure all are valid
+ *----------------------------------------------------------------------------
+ */
+BOOLEAN NlValidateAllAttrs(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+                           UINT32 totalAttrLen,
+                           const NL_POLICY policy[], const UINT32 numPolicy)
+{
+    PNL_ATTR nla;
+    INT left;
+    BOOLEAN ret = TRUE;
+
+    if ((NlMsgSize(nlMsg) < attrOffset)) {
+        OVS_LOG_WARN("No attributes in nlMsg: %p at offset: %d",
+            nlMsg, attrOffset);
+        ret = FALSE;
+        goto done;
+    }
+
+    NL_ATTR_FOR_EACH_UNSAFE(nla, left, NlMsgAt(nlMsg, attrOffset),
+                            totalAttrLen)
+    {
+        if (!NlAttrIsValid(nla, left)) {
+            ret = FALSE;
+            goto done;
+        }
+
+        UINT16 type = NlAttrType(nla);
+        if (type < numPolicy && policy[type].type != NL_A_NO_ATTR) {
+            /* Typecasting to keep the compiler happy */
+            const PNL_POLICY e = (const PNL_POLICY)(&policy[type]);
+            if (!NlAttrValidate(nla, e)) {
+                ret = FALSE;
+                goto done;
+            }
+        }
+    }
+
+done:
+    return ret;
+}
+
+/*
+ *----------------------------------------------------------------------------
  * Parses the netlink message at a given offset (attrOffset)
  * as a series of attributes. A pointer to the attribute with type
  * 'type' is stored in attrs at index 'type'. policy is used to define the
@@ -1052,20 +1105,13 @@ NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
             PNL_ATTR attrs[], UINT32 numAttrs)
 {
     PNL_ATTR nla;
-    UINT32 left;
+    INT left;
     UINT32 iter;
     BOOLEAN ret = FALSE;
     UINT32 numPolicyAttr = MIN(numPolicy, numAttrs);
 
     RtlZeroMemory(attrs, numAttrs * sizeof *attrs);
 
-
-    /* There is nothing to parse */
-    if (!(NlMsgAttrsLen(nlMsg))) {
-        ret = TRUE;
-        goto done;
-    }
-
     if ((NlMsgSize(nlMsg) < attrOffset)) {
         OVS_LOG_WARN("No attributes in nlMsg: %p at offset: %d",
                      nlMsg, attrOffset);
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h b/datapath-windows/ovsext/Netlink/Netlink.h
index 8f6a5bea5..63164c716 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -72,6 +72,7 @@ typedef struct _NL_POLICY
 /* This macro is careful to check for attributes with bad lengths. */
 #define NL_ATTR_FOR_EACH(ITER, LEFT, ATTRS, ATTRS_LEN)                  \
     for ((ITER) = (ATTRS), (LEFT) = (ATTRS_LEN);                        \
+         ((INT)LEFT) >= (INT)NLA_ALIGN(sizeof(NL_ATTR)) &&              \
          NlAttrIsValid(ITER, LEFT);                                     \
          (LEFT) -= NlAttrLenPad(ITER, LEFT), (ITER) = NlAttrNext(ITER))
 
@@ -80,7 +81,7 @@ typedef struct _NL_POLICY
  * already been validated (e.g. with NL_ATTR_FOR_EACH).  */
 #define NL_ATTR_FOR_EACH_UNSAFE(ITER, LEFT, ATTRS, ATTRS_LEN)           \
     for ((ITER) = (ATTRS), (LEFT) = (ATTRS_LEN);                        \
-         (LEFT) > 0;                                                    \
+         ((INT)LEFT) >= (INT)NLA_ALIGN(sizeof(NL_ATTR));                \
          (LEFT) -= NLA_ALIGN((ITER)->nlaLen), (ITER) = NlAttrNext(ITER))
 
 #define NL_ATTR_GET_AS(NLA, TYPE) \
@@ -94,8 +95,9 @@ BOOLEAN NlFillNlHdr(PNL_BUFFER nlBuf,
                     UINT16 nlmsgType, UINT16 nlmsgFlags,
                     UINT32 nlmsgSeq, UINT32 nlmsgPid);
 
-VOID NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgOut,
-                     UINT errorCode);
+VOID NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgError,
+                     UINT32 msgErrorLen,
+                     UINT errorCode, UINT32 *msgLen);
 
 /* Netlink message accessing the payload */
 PVOID NlMsgAt(const PNL_MSG_HDR nlh, UINT32 offset);
@@ -187,6 +189,11 @@ static __inline NlAttrIsLast(const PNL_ATTR nla, int rem)
 /* Netlink attribute validation */
 BOOLEAN NlAttrValidate(const PNL_ATTR, const PNL_POLICY);
 
+/* Netlink attribute stream validation */
+BOOLEAN NlValidateAllAttrs(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+                       UINT32 totalAttrLen,
+                       const NL_POLICY policy[], const UINT32 numPolicy);
+
 /* Put APis */
 BOOLEAN NlMsgPutNlHdr(PNL_BUFFER buf, PNL_MSG_HDR nlMsg);
 BOOLEAN NlMsgPutGenlHdr(PNL_BUFFER buf, PGENL_MSG_HDR genlMsg);