Thanks for adding in the CVE. Acked-by: Sairam Venugopal <[email protected]>
On 6/9/17, 7:54 PM, "[email protected] on behalf of Anand Kumar" <[email protected] on behalf of [email protected]> wrote: >- Minimum valid fragment size is 400 bytes, any fragment smaller >is likely to be intentionally crafted (CVE-2000-0305). > >- Validate maximum length of an Ip datagram > >- Added counters to keep track of number of fragments for a given >Ip datagram. > >Signed-off-by: Anand Kumar <[email protected]> >Acked-by: Alin Gabriel Serdean <[email protected]> >--- >v1->v2: Update commmit message to add (CVE-2000-0305) vulnerability >--- > datapath-windows/ovsext/Actions.c | 2 +- > datapath-windows/ovsext/IpFragment.c | 41 +++++++++++++++++++++++++----------- > datapath-windows/ovsext/IpFragment.h | 2 ++ > 3 files changed, 32 insertions(+), 13 deletions(-) > >diff --git a/datapath-windows/ovsext/Actions.c >b/datapath-windows/ovsext/Actions.c >index c3f0362..4eeaab3 100644 >--- a/datapath-windows/ovsext/Actions.c >+++ b/datapath-windows/ovsext/Actions.c >@@ -2030,7 +2030,7 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext, > if (status != NDIS_STATUS_SUCCESS) { > /* Pending NBLs are consumed by Defragmentation. */ > if (status != NDIS_STATUS_PENDING) { >- OVS_LOG_ERROR("CT Action failed"); >+ OVS_LOG_ERROR("CT Action failed status = %lu", status); > dropReason = L"OVS-conntrack action failed"; > } else { > /* We added a new pending NBL to be consumed later. >diff --git a/datapath-windows/ovsext/IpFragment.c >b/datapath-windows/ovsext/IpFragment.c >index 0874cb9..e601a15 100644 >--- a/datapath-windows/ovsext/IpFragment.c >+++ b/datapath-windows/ovsext/IpFragment.c >@@ -25,6 +25,10 @@ > #undef OVS_DBG_MOD > #endif > #define OVS_DBG_MOD OVS_DBG_IPFRAG >+/* Based on MIN_FRAGMENT_SIZE.*/ >+#define MAX_FRAGMENTS 164 >+#define MIN_FRAGMENT_SIZE 400 >+#define MAX_IPDATAGRAM_SIZE 65535 > > /* Function declarations */ > static VOID OvsIpFragmentEntryCleaner(PVOID data); >@@ -146,8 +150,9 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext, > IPHdr *ipHdr, *newIpHdr; > CHAR *ethBuf[sizeof(EthHdr)]; > CHAR *packetBuf; >- UINT16 ipHdrLen, packetLen, packetHeader; >+ UINT16 ipHdrLen, packetHeader; > POVS_FRAGMENT_LIST head = NULL; >+ UINT32 packetLen; > > curNb = NET_BUFFER_LIST_FIRST_NB(*curNbl); > ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL); >@@ -161,7 +166,10 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext, > if (ipHdr == NULL) { > return NDIS_STATUS_INVALID_PACKET; > } >- ipHdrLen = (UINT16)(ipHdr->ihl * 4); >+ ipHdrLen = ipHdr->ihl * 4; >+ if (ipHdrLen + entry->totalLen > MAX_IPDATAGRAM_SIZE) { >+ return NDIS_STATUS_INVALID_LENGTH; >+ } > packetLen = ETH_HEADER_LENGTH + ipHdrLen + entry->totalLen; > packetBuf = (CHAR*)OvsAllocateMemoryWithTag(packetLen, > OVS_IPFRAG_POOL_TAG); >@@ -185,7 +193,10 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext, > packetHeader = ETH_HEADER_LENGTH + ipHdrLen; > head = entry->head; > while (head) { >- ASSERT((packetHeader + head->offset) <= packetLen); >+ if ((UINT32)(packetHeader + head->offset) > packetLen) { >+ status = NDIS_STATUS_INVALID_DATA; >+ goto cleanup; >+ } > NdisMoveMemory(packetBuf + packetHeader + head->offset, > head->pbuff, head->len); > head = head->next; >@@ -197,10 +208,6 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext, > status = NDIS_STATUS_RESOURCES; > } > >- OvsFreeMemoryWithTag(packetBuf, OVS_IPFRAG_POOL_TAG); >- /* Timeout the entry so that clean up thread deletes it .*/ >- entry->expiration -= IPFRAG_ENTRY_TIMEOUT; >- > /* Complete the fragment NBL */ > ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(*curNbl); > if (ctx->flags & OVS_BUFFER_NEED_COMPLETE) { >@@ -214,6 +221,9 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext, > ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(*newNbl); > ctx->mru = entry->mru; > *curNbl = *newNbl; >+cleanup: >+ OvsFreeMemoryWithTag(packetBuf, OVS_IPFRAG_POOL_TAG); >+ entry->markedForDelete = TRUE; > return status; > } > /* >@@ -259,12 +269,15 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext, > if (ipHdr == NULL) { > return NDIS_STATUS_INVALID_PACKET; > } >- ipHdrLen = (UINT16)(ipHdr->ihl * 4); >+ ipHdrLen = ipHdr->ihl * 4; > payloadLen = ntohs(ipHdr->tot_len) - ipHdrLen; > offset = ntohs(ipHdr->frag_off) & IP_OFFSET; > offset <<= 3; > flags = ntohs(ipHdr->frag_off) & IP_MF; >- >+ /* Only the last fragment can be of smaller size.*/ >+ if (flags && ntohs(ipHdr->tot_len) < MIN_FRAGMENT_SIZE) { >+ return NDIS_STATUS_INVALID_LENGTH; >+ } > /*Copy fragment specific fields. */ > fragKey.protocol = ipHdr->protocol; > fragKey.id = ipHdr->id; >@@ -318,6 +331,7 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext, > entry->mru = ETH_HEADER_LENGTH + ipHdrLen + payloadLen; > entry->recvdLen += fragStorage->len; > entry->head = entry->tail = fragStorage; >+ entry->numFragments = 1; > if (!flags) { > entry->totalLen = offset + payloadLen; > } >@@ -337,8 +351,9 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext, > /* Acquire the entry lock. */ > NdisAcquireSpinLock(&(entry->lockObj)); > NdisGetCurrentSystemTime((LARGE_INTEGER *)¤tTime); >- if (currentTime > entry->expiration) { >- /* Expired entry. */ >+ if (currentTime > entry->expiration || entry->numFragments == >MAX_FRAGMENTS) { >+ /* Mark the entry for delete. */ >+ entry->markedForDelete = TRUE; > goto fragment_error; > } > POVS_FRAGMENT_LIST next = entry->head; >@@ -393,6 +408,7 @@ found: > /*Update Maximum recieved Unit */ > entry->mru = entry->mru > (ETH_HEADER_LENGTH + ipHdrLen + payloadLen) > ? > entry->mru : (ETH_HEADER_LENGTH + ipHdrLen + payloadLen); >+ entry->numFragments++; > if (!flags) { > entry->totalLen = offset + payloadLen; > } >@@ -404,6 +420,7 @@ found: > return status; > } > fragment_error: >+ status = NDIS_STATUS_INVALID_PACKET; > /* Release the entry lock. */ > NdisReleaseSpinLock(&(entry->lockObj)); > payload_copy_error: >@@ -460,7 +477,7 @@ static VOID > OvsIpFragmentEntryDelete(POVS_IPFRAG_ENTRY entry, BOOLEAN checkExpiry) > { > NdisAcquireSpinLock(&(entry->lockObj)); >- if (checkExpiry) { >+ if (!entry->markedForDelete && checkExpiry) { > UINT64 currentTime; > NdisGetCurrentSystemTime((LARGE_INTEGER *)¤tTime); > if (entry->expiration > currentTime) { >diff --git a/datapath-windows/ovsext/IpFragment.h >b/datapath-windows/ovsext/IpFragment.h >index e650399..cd5b960 100644 >--- a/datapath-windows/ovsext/IpFragment.h >+++ b/datapath-windows/ovsext/IpFragment.h >@@ -37,6 +37,8 @@ typedef struct _OVS_IPFRAG_KEY { > > typedef struct _OVS_IPFRAG_ENTRY { > NDIS_SPIN_LOCK lockObj; /* To access the entry. */ >+ BOOLEAN markedForDelete; >+ UINT8 numFragments; > UINT16 totalLen; > UINT16 recvdLen; > UINT16 mru; >-- >2.9.3.windows.1 > >_______________________________________________ >dev mailing list >[email protected] >https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=Fj48Qs5cvjKoU6eV7J_szN5n4VTBzbIRnT73MY8sNNk&s=if3mZ38TqK_TYmpMtaFl8qGdV9_IWeG-omGU3q0X-Ho&e= > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
