ambind: fix unsecure use of FD_SET()#225
Open
jeffbencteux wants to merge 1 commit intozmanda:masterfrom
Open
Conversation
ambind does not check user-provided fd value given on its command-line. This fd is then used in calls to FD_SET() and then select(). from man(3) fd_set: > An fd_set is a fixed size buffer. Executing FD_CLR() or FD_SET() with a value of fd that is negative or is equal to or larger than FD_SETSIZE will result in undefined > behavior. Moreover, POSIX requires fd to be a valid file descriptor. For example, it can be triggered with an int overflow: $ LD_LIBRARY_PATH=./common-src/.libs/:"$LD_LIBRARY_PATH" ./common-src/.libs/ambind 2147483648 zsh: segmentation fault LD_LIBRARY_PATH=./common-src/.libs/:"$LD_LIBRARY_PATH" 2147483648 ltrace stack: $ LD_LIBRARY_PATH=./common-src/.libs/:"$LD_LIBRARY_PATH" ltrace ./common-src/.libs/ambind 2147483648 atoi(0x7ffefcb73354, 0x7ffefcb728e8, 0x7ffefcb72900, 0x55797923ddb8) = 0x80000000 --- SIGSEGV (Segmentation fault) --- +++ killed by SIGSEGV +++ Current patch fix this behaviour by checking for fd being in range ]0;FD_SETSIZE[. Note that FD_SETSIZE is often 1024 but it is not guaranteed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ambind does not check user-provided fd value given on its command-line. This fd is then used in calls to
FD_SET()and thenselect().from
man(3) fd_set:For example, it can be triggered with an int overflow:
ltrace stack:
Current patch fix this behaviour by checking for fd being in range
]0;FD_SETSIZE[. Note thatFD_SETSIZEis often 1024 but it is not guaranteed.I am unsure of the security implications of the misuse. ambind is currently SUID root on my debian. However I do not have a clear picture of how that binary is typically used and how often. It is possible that it can lead to denial of service (DoS) if an attacker can provide the port to bind to but I doubt exploitation could go further. I am interested in getting your feedback on that.
Similar issue in other projects:
https://www.chiark.greenend.org.uk/~sgtatham/putty/wishlist/vuln-fd-set-overflow.html
https://www.cvedetails.com/cve/CVE-2011-0719/