Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

ulDNSHandlePacket() - check length before calculating payload size #1615

Merged
merged 1 commit into from
Dec 13, 2019
Merged

ulDNSHandlePacket() - check length before calculating payload size #1615

merged 1 commit into from
Dec 13, 2019

Conversation

gkwicker
Copy link
Contributor

Detect integer overflow in ulDNSHandlePacket().


This PR fixes an integer overflow case introduced in an earlier commit and detected by a failing ulDNSHandlePacket() test. See here for another example of this overflow detection logic.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • My code is Linted.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

uxPayloadSize,
pdFALSE );
}
}

/* The packet was not consumed. */
return pdFAIL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call me crazy, but why have a return value if it can never be anything other than pdFAIL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though this handler never consumes the buffer, the IP stack expects handling code to return a status value indicating whether or not the buffer was consumed.

@gkwicker gkwicker merged commit d82b44f into aws:release-candidate Dec 13, 2019
@htibosch
Copy link
Contributor

Hello @gkwicker I think that your change doesn't add anything.
Note my comment in the source code:

The value of 'xDataLength' was proven to be at least the size of a
UDP packet in prvProcessIPPacket()

When xProcessReceivedUDPPacket() is called, you don't need to test this again:

    if( pxNetworkBuffer->xDataLength >= sizeof( UDPPacket_t ) )

When you follow the flow of the program, you will see that the statement is always true.

Now for the call-back ( a rarely used feature in FreeRTOS+TCP ), I had forgotten to subtract the size of the UDP packet. I think that you can safely drop this PR #1615.

Thanks, Hein

VanNamDinh pushed a commit to renesas/amazon-freertos that referenced this pull request Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants