Trap for overly large input to XmlRPCPP #2065
Merged
Conversation
This file contains 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
…th int <-> size_t conversions. - In XmlRpcClient, XmlRpcServerConnection and XmlRpcSocket, recognize when incoming or outgoing data is too large, generate an error and discard the data when practical. - Use the safe strtol() rather than atoi() to decode an incoming content-length header, and generate an error if the length is invalid or too large. - In XmlRpcUtil, prevent attempts to parse overly large XML input. - Add tests where they can reasonably be inserted into existing test routines. Although this fix could be cleaner the update is written to make the update ABI compatible. This fix addresses CVE-2020-16124 / Integer overflow in ros_comm. Signed-off-by: Sid Faber <sid.faber@canonical.com>
dirk-thomas
reviewed
Sep 30, 2020
@dirk-thomas, thanks for the review. The overrun tests I added are causing memory allocation failures--they work on my local machine but I assume the CI tests don't expect to be handling 2Gb input values. Any suggestions on how best to handle, is it advisable to remove the overrun tests? |
Can the code be written in a way that it fails gracefully if that much memory isn't available? Maybe marking the test as skipped. |
Signed-off-by: Sid Faber <sid.faber@canonical.com>
Signed-off-by: Sid Faber <sid.faber@canonical.com>
|
Debian Buster only has Googletest 1.8.1 which doesn't have The |
Replace call to GTEST_SKIP with output to stderr. Remove the redResponseOversize test since out-of-memory errors during testing cannot easily be handled within the existing test objects. Signed-off-by: Sid Faber <sid.faber@canonical.com>
|
I removed GTEST_SKIP, printed an error message but otherwise let the test succeed. Also I removed one test that was causing problems. The test_client routines for readResponse still run when objects are finalized; since the test object could be in an unpredictable state I couldn't find a good way to ignore the results. |
That sounds like a good fall backback behavior. Can you make this conditional based on if |
Use GTEST_SKIP if available, otherwise print to stderr. Remove test that's being killed because it takes too long to handle the oversize test values
SidFaber
force-pushed
the
bugfix/size_t
branch
from
b890d3f
to
917e880
Compare
|
@dirk-thomas, I added the conditional statement in one of the tests but had to rip another one out--the Ubuntu tests started running long and got killed causing the whole module to fail. I think this is as good as I can get it unless you have any ideas on how to safely & consistently write tests for huge input strings. |
|
Sounds good to me. Thanks for the patch and for iterating on it. |
jacobperron
pushed a commit
that referenced
this pull request
Oct 16, 2020
* Trap for overly large input to XmlRPCPP which could cause problems with int <-> size_t conversions. - In XmlRpcClient, XmlRpcServerConnection and XmlRpcSocket, recognize when incoming or outgoing data is too large, generate an error and discard the data when practical. - Use the safe strtol() rather than atoi() to decode an incoming content-length header, and generate an error if the length is invalid or too large. - In XmlRpcUtil, prevent attempts to parse overly large XML input. - Add tests where they can reasonably be inserted into existing test routines. Although this fix could be cleaner the update is written to make the update ABI compatible. This fix addresses CVE-2020-16124 / Integer overflow in ros_comm. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Trap for memory allocation error in tests Signed-off-by: Sid Faber <sid.faber@canonical.com> * Revert earlier change Signed-off-by: Sid Faber <sid.faber@canonical.com> * Update tests Replace call to GTEST_SKIP with output to stderr. Remove the redResponseOversize test since out-of-memory errors during testing cannot easily be handled within the existing test objects. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Improve test error handling Use GTEST_SKIP if available, otherwise print to stderr. Remove test that's being killed because it takes too long to handle the oversize test values
jacobperron
pushed a commit
that referenced
this pull request
Oct 22, 2020
* Trap for overly large input to XmlRPCPP which could cause problems with int <-> size_t conversions. - In XmlRpcClient, XmlRpcServerConnection and XmlRpcSocket, recognize when incoming or outgoing data is too large, generate an error and discard the data when practical. - Use the safe strtol() rather than atoi() to decode an incoming content-length header, and generate an error if the length is invalid or too large. - In XmlRpcUtil, prevent attempts to parse overly large XML input. - Add tests where they can reasonably be inserted into existing test routines. Although this fix could be cleaner the update is written to make the update ABI compatible. This fix addresses CVE-2020-16124 / Integer overflow in ros_comm. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Trap for memory allocation error in tests Signed-off-by: Sid Faber <sid.faber@canonical.com> * Revert earlier change Signed-off-by: Sid Faber <sid.faber@canonical.com> * Update tests Replace call to GTEST_SKIP with output to stderr. Remove the redResponseOversize test since out-of-memory errors during testing cannot easily be handled within the existing test objects. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Improve test error handling Use GTEST_SKIP if available, otherwise print to stderr. Remove test that's being killed because it takes too long to handle the oversize test values
jacobperron
pushed a commit
that referenced
this pull request
Oct 22, 2020
* Trap for overly large input to XmlRPCPP which could cause problems with int <-> size_t conversions. - In XmlRpcClient, XmlRpcServerConnection and XmlRpcSocket, recognize when incoming or outgoing data is too large, generate an error and discard the data when practical. - Use the safe strtol() rather than atoi() to decode an incoming content-length header, and generate an error if the length is invalid or too large. - In XmlRpcUtil, prevent attempts to parse overly large XML input. - Add tests where they can reasonably be inserted into existing test routines. Although this fix could be cleaner the update is written to make the update ABI compatible. This fix addresses CVE-2020-16124 / Integer overflow in ros_comm. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Trap for memory allocation error in tests Signed-off-by: Sid Faber <sid.faber@canonical.com> * Revert earlier change Signed-off-by: Sid Faber <sid.faber@canonical.com> * Update tests Replace call to GTEST_SKIP with output to stderr. Remove the redResponseOversize test since out-of-memory errors during testing cannot easily be handled within the existing test objects. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Improve test error handling Use GTEST_SKIP if available, otherwise print to stderr. Remove test that's being killed because it takes too long to handle the oversize test values
jacobperron
pushed a commit
that referenced
this pull request
Oct 22, 2020
* Trap for overly large input to XmlRPCPP which could cause problems with int <-> size_t conversions. - In XmlRpcClient, XmlRpcServerConnection and XmlRpcSocket, recognize when incoming or outgoing data is too large, generate an error and discard the data when practical. - Use the safe strtol() rather than atoi() to decode an incoming content-length header, and generate an error if the length is invalid or too large. - In XmlRpcUtil, prevent attempts to parse overly large XML input. - Add tests where they can reasonably be inserted into existing test routines. Although this fix could be cleaner the update is written to make the update ABI compatible. This fix addresses CVE-2020-16124 / Integer overflow in ros_comm. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Trap for memory allocation error in tests Signed-off-by: Sid Faber <sid.faber@canonical.com> * Revert earlier change Signed-off-by: Sid Faber <sid.faber@canonical.com> * Update tests Replace call to GTEST_SKIP with output to stderr. Remove the redResponseOversize test since out-of-memory errors during testing cannot easily be handled within the existing test objects. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Improve test error handling Use GTEST_SKIP if available, otherwise print to stderr. Remove test that's being killed because it takes too long to handle the oversize test values
jacobperron
pushed a commit
that referenced
this pull request
Oct 22, 2020
* Trap for overly large input to XmlRPCPP which could cause problems with int <-> size_t conversions. - In XmlRpcClient, XmlRpcServerConnection and XmlRpcSocket, recognize when incoming or outgoing data is too large, generate an error and discard the data when practical. - Use the safe strtol() rather than atoi() to decode an incoming content-length header, and generate an error if the length is invalid or too large. - In XmlRpcUtil, prevent attempts to parse overly large XML input. - Add tests where they can reasonably be inserted into existing test routines. Although this fix could be cleaner the update is written to make the update ABI compatible. This fix addresses CVE-2020-16124 / Integer overflow in ros_comm. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Trap for memory allocation error in tests Signed-off-by: Sid Faber <sid.faber@canonical.com> * Revert earlier change Signed-off-by: Sid Faber <sid.faber@canonical.com> * Update tests Replace call to GTEST_SKIP with output to stderr. Remove the redResponseOversize test since out-of-memory errors during testing cannot easily be handled within the existing test objects. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Improve test error handling Use GTEST_SKIP if available, otherwise print to stderr. Remove test that's being killed because it takes too long to handle the oversize test values
jacobperron
pushed a commit
that referenced
this pull request
Oct 22, 2020
* Trap for overly large input to XmlRPCPP which could cause problems with int <-> size_t conversions. - In XmlRpcClient, XmlRpcServerConnection and XmlRpcSocket, recognize when incoming or outgoing data is too large, generate an error and discard the data when practical. - Use the safe strtol() rather than atoi() to decode an incoming content-length header, and generate an error if the length is invalid or too large. - In XmlRpcUtil, prevent attempts to parse overly large XML input. - Add tests where they can reasonably be inserted into existing test routines. Although this fix could be cleaner the update is written to make the update ABI compatible. This fix addresses CVE-2020-16124 / Integer overflow in ros_comm. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Trap for memory allocation error in tests Signed-off-by: Sid Faber <sid.faber@canonical.com> * Revert earlier change Signed-off-by: Sid Faber <sid.faber@canonical.com> * Update tests Replace call to GTEST_SKIP with output to stderr. Remove the redResponseOversize test since out-of-memory errors during testing cannot easily be handled within the existing test objects. Signed-off-by: Sid Faber <sid.faber@canonical.com> * Improve test error handling Use GTEST_SKIP if available, otherwise print to stderr. Remove test that's being killed because it takes too long to handle the oversize test values
nim65s
added a commit
to nim65s/robotpkg
that referenced
this pull request
Mar 12, 2021
Because DEPEND_ABI.ros-comm.noetic?= ros-comm>=1.15 1.15.9 (2020-10-16) ------------------- * Fix deadlock when service connection is dropped (ros/ros_comm#2074) * Update maintainers (ros/ros_comm#2075) * Fix case where accessing cached parameters shuts down another node (ros/ros_comm#2068) * Fix spelling (ros/ros_comm#2066) * Fix Lost Wake Bug in ROSOutAppender (ros/ros_comm#2033) * Fix compatibility issue with boost 1.73 and above (ros/ros_comm#2023) * Gracefully stop recording upon SIGTERM and SIGINT (ros/ros_comm#2038) * Use heapq.merge instead of custom merge sort code (ros/ros_comm#2017) * Fix handling of single quotes in command arguments on Windows (ros/ros_comm#2051) * Clearer error message (ros/ros_comm#2035) * Ignore underscores when parsing literal numeric values for Python 3 compatibility (ros/ros_comm#2022) * Clear cached URI for a node that has gone offline (ros/ros_comm#2010) * Add skip_cache parameter to rosnode_ping() (ros/ros_comm#2009) * Install advertisetest (ros/ros_comm#2046) * Use range instead of xrange for Python 3 compatibility (ros/ros_comm#2013) * Fix to address CVE-2020-16124 (ros/ros_comm#2065) * Fix XmlRpcValue::_doubleFormat being unused (ros/ros_comm#2003) 1.15.8 (2020-07-23) ------------------- * change is_async_connected to use epoll when available (ros/ros_comm#1983) * allow mixing latched and unlatched publishers (ros/ros_comm#1991) * remove not existing NodeProxy from rospy __all_\_ (ros/ros_comm#2007) * fix typo in topics.py (ros/ros_comm#1977) * fix bad relative import (still Python 2 style) (ros/ros_comm#1973) * improve shutdown message with duplicate node name (ros/ros_comm#1992) * remove dependency on rostopic from rostest package (ros/ros_comm#2002) * fix missing reload() function in Python 3 (ros/ros_comm#1968) * add latch param to throttle (ros/ros_comm#1944) * add const versions of XmlRpcValue converting operators (ros/ros_comm#1978) 1.15.7 (2020-05-28) ------------------- * fix Windows build break (ros/ros_comm#1961) * fix NameError in launch error handling (ros/ros_comm#1965) 1.15.6 (2020-05-21) ------------------- * fix a bug that using a destroyed connection object (ros/ros_comm#1950) 1.15.5 (2020-05-15) ------------------- * check if async socket connect is success or failure before TransportTCP::read() and TransportTCP::write() (ros/ros_comm#1954) * fix bug that connection drop signal related funtion throw a bad_weak exception (ros/ros_comm#1940) * multiple latched publishers per process on the same topic (ros/ros_comm#1544) * fix negative numbers in ros statistics (ros/ros_comm#1531) * remove extra \n in ROS_DEBUG (ros/ros_comm#1925) * add option to repeat latched messages at the start of bag splits (ros/ros_comm#1850) * fix bag migration failures caused by typo in connection_header assignment (ros/ros_comm#1952) * fix brief description comments after members (ros/ros_comm#1920) * add --sigint-timeout and --sigterm-timeout parameters (ros/ros_comm#1937) * roslaunch-check: search dir recursively (ros/ros_comm#1914) * sort printed nodes by namespace alphabetically (ros/ros_comm#1934) * remove pycrypto import (not used) (ros/ros_comm#1922) * avoid infinite recursion in rosrun tab completion when rosbash is not installed (ros/ros_comm#1948) * fix bare pointer in topic_tools::ShapeShifter (ros/ros_comm#1722) * clear message queue on simtime jumping back (ros/ros_comm#1518) * use undefined dynamic_lookup on macOS (ros/ros_comm#1923) * check if enough FDs are free, instead counting the total free FDs (ros/ros_comm#1929) 1.15.4 (2020-03-19) ------------------- * restrict boost dependencies to components used (ros/ros_comm#1871) * add exception for ConnectionAbortedError (ros/ros_comm#1908) * fix mac trying to use epoll instead of kqueue (ros/ros_comm#1907) * fix AttributeError: __exit__ (ros/ros_comm#1915, regression from 1.14.4) 1.15.3 (2020-02-28) ------------------- * remove Boost version check since Noetic only targets platforms with 1.67+ (ros/ros_comm#1903) 1.15.2 (2020-02-25) ------------------- * export missing Boost dependency (ros/ros_comm#1898) * add timestamp formatting for rosconsole (ros/ros_comm#1892) 1.15.1 (2020-02-24) ------------------- * fix missing boost dependencies (ros/ros_comm#1895) * use setuptools instead of distutils (ros/ros_comm#1870) * increase time limit of advertisetest/publishtest.test to reduce flakyness (ros/ros_comm#1897) 1.15.0 (2020-02-21) ------------------- * fix dictionary changed size during iteration (ros/ros_comm#1894) * update test to pass with old and new yaml (ros/ros_comm#1893) Packaging changes: - removed patch-an, as there are no more boost version checks - updated patch-ao
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Oversize input to XmlRPCPP could cause problems with int <-> size_t conversions.
Although this fix could be cleaner the update is written to make the update ABI compatible.
This fix addresses CVE-2020-16124 / Integer overflow in ros_comm.
Signed-off-by: Sid Faber sid.faber@canonical.com