From e6b298ef73816355e94e22a7df0cd28414957087 Mon Sep 17 00:00:00 2001 From: Paul Boca Date: Wed, 27 Apr 2016 08:05:47 +0000 Subject: 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 Acked-by: Alin Gabriel Serdean Signed-off-by: Ben Pfaff --- datapath-windows/ovsext/Flow.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) (limited to 'datapath-windows/ovsext/Flow.c') diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c index 1f236257e..d957d3962 100644 --- a/datapath-windows/ovsext/Flow.c +++ b/datapath-windows/ovsext/Flow.c @@ -285,20 +285,10 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, goto done; } - /* Get all the top level Flow attributes */ - if ((NlAttrParse(nlMsgHdr, attrOffset, NlMsgAttrsLen(nlMsgHdr), - nlFlowPolicy, ARRAY_SIZE(nlFlowPolicy), - flowAttrs, ARRAY_SIZE(flowAttrs))) - != TRUE) { - OVS_LOG_ERROR("Attr Parsing failed for msg: %p", - nlMsgHdr); - rc = STATUS_INVALID_PARAMETER; - goto done; - } - - /* FLOW_DEL command w/o any key input is a flush case. */ + /* FLOW_DEL command w/o any key input is a flush case. + If we don't have any attr, we treat this as a flush command*/ if ((genlMsgHdr->cmd == OVS_FLOW_CMD_DEL) && - (!(flowAttrs[OVS_FLOW_ATTR_KEY]))) { + (!NlMsgAttrsLen(nlMsgHdr))) { rc = OvsFlushFlowIoctl(ovsHdr->dp_ifindex); @@ -323,6 +313,17 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, goto done; } + /* Get all the top level Flow attributes */ + if ((NlAttrParse(nlMsgHdr, attrOffset, NlMsgAttrsLen(nlMsgHdr), + nlFlowPolicy, ARRAY_SIZE(nlFlowPolicy), + flowAttrs, ARRAY_SIZE(flowAttrs))) + != TRUE) { + OVS_LOG_ERROR("Attr Parsing failed for msg: %p", + nlMsgHdr); + rc = STATUS_INVALID_PARAMETER; + goto done; + } + if (flowAttrs[OVS_FLOW_ATTR_PROBE]) { rc = OvsProbeSupportedFeature(msgIn, flowAttrs[OVS_FLOW_ATTR_KEY]); if (rc != STATUS_SUCCESS) { @@ -399,8 +400,9 @@ done: if (nlError != NL_ERROR_SUCCESS) { POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - NlBuildErrorMsg(msgIn, msgError, nlError); - *replyLen = msgError->nlMsg.nlmsgLen; + UINT32 msgErrorLen = usrParamsCtx->outputLength; + + NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); rc = STATUS_SUCCESS; } @@ -568,8 +570,9 @@ done: if (nlError != NL_ERROR_SUCCESS) { POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - NlBuildErrorMsg(msgIn, msgError, nlError); - *replyLen = msgError->nlMsg.nlmsgLen; + UINT32 msgErrorLen = usrParamsCtx->outputLength; + + NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); rc = STATUS_SUCCESS; } @@ -706,8 +709,9 @@ done: if (nlError != NL_ERROR_SUCCESS) { POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - NlBuildErrorMsg(msgIn, msgError, nlError); - *replyLen = msgError->nlMsg.nlmsgLen; + UINT32 msgErrorLen = usrParamsCtx->outputLength; + + NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); rc = STATUS_SUCCESS; } -- cgit v1.2.1