Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix out-of-bounds uIP data pointer on some TCP URG segments #1173

Open
wants to merge 1 commit into
base: develop
from

Conversation

@TheJokr
Copy link

TheJokr commented Jan 9, 2020

This is a bug/vulnerability I discovered while working on a paper on TCP specification conformance. One of the implementations we tested was uIP (standalone v1.0, so quite old), which generated segmentation faults when we ran a test regarding TCP's urgent pointer. We determined the cause within uIP (see below), but didn't submit a fix for it initially because we thought the project was inactive (judging from its GitHub repo).

If the UIP_URGDATA macro is zero (the default), the amount of urgent data is deducted from the uip_appdata pointer and from uip_len without any size check. This can cause out-of-bounds reads (for the former) and unsigned integer underflow (for the latter) when the urgent pointer points past the current segment.

Now that we discovered its use within contiki(-ng), the (quite simple) fix in this PR is to cap the effective urgent data length at urg_len independent of UIP_URGDATA's value. The already-present tmp16 variable is used to hold the length in the absence of uip_urglen on builds with UIP_URGDAT == 0.

If UIP_URGDATA is zero (the default), the amount of urgent data is
deducted from the uip_appdata pointer and from uip_len without a size
check. This can cause out-of-bounds reads (for the former) and unsigned
integer underflow (for the latter).

To fix the vulnerability, the urgent data length is checked against
urg_len independent of the value of UIP_URGDATA and bounded
appropriately.
@yatch yatch self-assigned this Jan 9, 2020
@yatch

This comment has been minimized.

Copy link
Member

yatch commented Jan 9, 2020

@TheJokr Great 👍 I'm going to review the current TCP urgent mechanism implementation as well.

This is a bug/vulnerability I discovered while working on a paper on TCP specification conformance.

Is the paper publicly available? I'm interested.

@TheJokr

This comment has been minimized.

Copy link
Author

TheJokr commented Jan 10, 2020

Is the paper publicly available? I'm interested.

Awesome! It's not quite finished yet, but it got accepted at PAM 2020. I'll leave a comment here once the preprint is available on arXiv.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.