Skip to content

Commit

Permalink
Fix asset_uri_whitelist regex backtracking issue, add more extensio…
Browse files Browse the repository at this point in the history
…ns (#270)

### Public-Facing Changes

Fix exponential regex backtracking issue with default
`asset_uri_whitelist`

### Description
When fetching the asset
`package://foxglove_simulation/meshes/kinect_jpg` I noticed that I never
got a response. It turned out that the reason for this was due to
catastrophic backtracking caused by the default `asset_uri_whitelist`
regular expression pattern. The default pattern contains nested
quantifiers with alternations, which can lead to exponential
backtracking. This can cause the program to hang or freeze when trying
to match certain input strings.

This PR changes the default `asset_uri_whitelist` regex pattern to avoid
catastrophic backtracking. It additionally adds a couple more extensions
to e.g. allow for retrieval of texture files.
  • Loading branch information
achim-k authored Oct 25, 2023
1 parent e52bb9d commit 4af6fa2
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 7 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Parameters are provided to configure the behavior of the bridge. These parameter
* __send_buffer_limit__: Connection send buffer limit in bytes. Messages will be dropped when a connection's send buffer reaches this limit to avoid a queue of outdated messages building up. Defaults to `10000000` (10 MB).
* __use_compression__: Use websocket compression (permessage-deflate). Suited for connections with smaller bandwith, at the cost of additional CPU load.
* __capabilities__: List of supported [server capabilities](https://github.com/foxglove/ws-protocol/blob/main/docs/spec.md). Defaults to `[clientPublish,parameters,parametersSubscribe,services,connectionGraph,assets]`.
* __asset_uri_allowlist__: List of regular expressions ([ECMAScript grammar](https://en.cppreference.com/w/cpp/regex/ecmascript)) of allowed asset URIs. Uses the [resource_retriever](https://index.ros.org/p/resource_retriever/github-ros-resource_retriever) to resolve `package://`, `file://` or `http(s)://` URIs. Note that this list should be carefully configured such that no confidential files are accidentally exposed over the websocket connection. As an extra security measure, URIs containing two consecutive dots (`..`) are disallowed as they could be used to construct URIs that would allow retrieval of confidential files if the allowlist is not configured strict enough (e.g. `package://<pkg_name>/../../../secret.txt`). Defaults to `["package://(\w+/?)+\.(dae|stl|urdf|xacro)"]`.
* __asset_uri_allowlist__: List of regular expressions ([ECMAScript grammar](https://en.cppreference.com/w/cpp/regex/ecmascript)) of allowed asset URIs. Uses the [resource_retriever](https://index.ros.org/p/resource_retriever/github-ros-resource_retriever) to resolve `package://`, `file://` or `http(s)://` URIs. Note that this list should be carefully configured such that no confidential files are accidentally exposed over the websocket connection. As an extra security measure, URIs containing two consecutive dots (`..`) are disallowed as they could be used to construct URIs that would allow retrieval of confidential files if the allowlist is not configured strict enough (e.g. `package://<pkg_name>/../../../secret.txt`). Defaults to `["^package://(?:\w+/)*\w+\.(?:dae|fbx|glb|gltf|jpeg|jpg|mtl|obj|png|stl|tif|tiff|urdf|webp|xacro)$"]`.
* (ROS 1) __max_update_ms__: The maximum number of milliseconds to wait in between polling `roscore` for new topics, services, or parameters. Defaults to `5000`.
* (ROS 1) __service_type_retrieval_timeout_ms__: Max number of milliseconds for retrieving a services type information. Defaults to `250`.
* (ROS 2) __num_threads__: The number of threads to use for the ROS node executor. This controls the number of subscriptions that can be processed in parallel. 0 means one thread per CPU core. Defaults to `0`.
Expand Down
2 changes: 1 addition & 1 deletion ros1_foxglove_bridge/launch/foxglove_bridge.launch
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<arg name="nodelet_manager" default="foxglove_nodelet_manager" />
<arg name="num_threads" default="0" />
<arg name="capabilities" default="[clientPublish,parameters,parametersSubscribe,services,connectionGraph,assets]" />
<arg name="asset_uri_allowlist" default="['package://(/?\w+)+\.(dae|stl|urdf|xacro)']" />
<arg name="asset_uri_allowlist" default="['^package://(?:\w+/)*\w+\.(?:dae|fbx|glb|gltf|jpeg|jpg|mtl|obj|png|stl|tif|tiff|urdf|webp|xacro)$']" />
<arg name="service_type_retrieval_timeout_ms" default="250" />

<node pkg="nodelet" type="nodelet" name="foxglove_nodelet_manager" args="manager"
Expand Down
4 changes: 3 additions & 1 deletion ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ class FoxgloveBridge : public nodelet::Nodelet {
}

const auto assetUriAllowlist = nhp.param<std::vector<std::string>>(
"asset_uri_allowlist", {"package://(/?\\w+)+\\.(dae|stl|urdf|xacro)"});
"asset_uri_allowlist",
{"^package://(?:\\w+/"
")*\\w+\\.(?:dae|fbx|glb|gltf|jpeg|jpg|mtl|obj|png|stl|tif|tiff|urdf|webp|xacro)$"});
_assetUriAllowlistPatterns = parseRegexPatterns(assetUriAllowlist);
if (assetUriAllowlist.size() != _assetUriAllowlistPatterns.size()) {
ROS_ERROR("Failed to parse one or more asset URI whitelist patterns");
Expand Down
2 changes: 1 addition & 1 deletion ros2_foxglove_bridge/launch/foxglove_bridge_launch.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<arg name="use_sim_time" default="false" />
<arg name="capabilities" default="[clientPublish,parameters,parametersSubscribe,services,connectionGraph,assets]" />
<arg name="include_hidden" default="false" />
<arg name="asset_uri_allowlist" default="['package://(\\w+/?)+\\.(dae|stl|urdf|xacro)']" /> <!-- Needs double-escape -->
<arg name="asset_uri_allowlist" default="['^package://(?:\\w+/)*\\w+\\.(?:dae|fbx|glb|gltf|jpeg|jpg|mtl|obj|png|stl|tif|tiff|urdf|webp|xacro)$']" /> <!-- Needs double-escape -->

<node pkg="foxglove_bridge" exec="foxglove_bridge">
<param name="port" value="$(var port)" />
Expand Down
9 changes: 6 additions & 3 deletions ros2_foxglove_bridge/src/param_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,12 @@ void declareParameters(rclcpp::Node* node) {
assetUriAllowlistDescription.description =
"List of regular expressions (ECMAScript) of whitelisted asset URIs.";
assetUriAllowlistDescription.read_only = true;
node->declare_parameter(PARAM_ASSET_URI_ALLOWLIST,
std::vector<std::string>({"package://(/?\\w+)+\\.(dae|stl|urdf|xacro)"}),
paramWhiteListDescription);
node->declare_parameter(
PARAM_ASSET_URI_ALLOWLIST,
std::vector<std::string>(
{"^package://(?:\\w+/"
")*\\w+\\.(?:dae|fbx|glb|gltf|jpeg|jpg|mtl|obj|png|stl|tif|tiff|urdf|webp|xacro)$"}),
paramWhiteListDescription);
}

std::vector<std::regex> parseRegexStrings(rclcpp::Node* node,
Expand Down

0 comments on commit 4af6fa2

Please sign in to comment.