Skip to content

Commit

Permalink
Merge pull request #12280 from RasaHQ/ATO-877-Snyk-report-Regular-Exp…
Browse files Browse the repository at this point in the history
…ression-Denial-of-Service-on-Slack-connector

Address Regular Expression Denial of Service vulnerability
  • Loading branch information
vcidst committed Apr 21, 2023
2 parents f656a86 + 5b2d232 commit 319811f
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 8 deletions.
1 change: 1 addition & 0 deletions changelog/12280.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Addresses Regular Expression Denial of Service vulnerability in slack connector (https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS)
11 changes: 6 additions & 5 deletions rasa/core/channels/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ def __init__(
thread_id: Optional[Text] = None,
proxy: Optional[Text] = None,
) -> None:

self.slack_channel = slack_channel
self.thread_id = thread_id
self.proxy = proxy
Expand Down Expand Up @@ -268,21 +267,23 @@ def _sanitize_user_message(
uids_to_remove = uids_to_remove or []

for uid_to_remove in uids_to_remove:
escaped_uid = re.escape(str(uid_to_remove))

# heuristic to format majority cases OK
# can be adjusted to taste later if needed,
# but is a good first approximation
for regex, replacement in [
(rf"<@{uid_to_remove}>\s", ""),
(rf"\s<@{uid_to_remove}>", ""), # a bit arbitrary but probably OK
(rf"<@{uid_to_remove}>", " "),
(rf"<@{escaped_uid}>\s", ""),
(rf"\s<@{escaped_uid}>", ""), # a bit arbitrary but probably OK
(rf"<@{escaped_uid}>", " "),
]:
text = re.sub(regex, replacement, text)

# Find multiple mailto or http links like
# <mailto:xyz@rasa.com|xyz@rasa.com> or
# <http://url.com|url.com> in text and substitute
# it with original content
pattern = r"(\<(?:mailto|http|https):\/\/.*?\|.*?\>)"
pattern = r"(\<(?:mailto|https?):\/\/.*?\|.*?\>)"
match = re.findall(pattern, text)

if match:
Expand Down
15 changes: 12 additions & 3 deletions tests/core/channels/test_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import time
from typing import Any, Dict, Text
from unittest import mock
from unittest.mock import Mock
from unittest.mock import Mock, patch

from aioresponses import aioresponses
import pytest
Expand Down Expand Up @@ -248,7 +248,7 @@ def test_slack_no_metadata():


def test_slack_message_sanitization():
test_uid = 17213535
test_uid = "17213535"
target_message_1 = "You can sit here if you want"
target_message_2 = "Hey, you can sit here if you want !"
target_message_3 = "Hey, you can sit here if you want!"
Expand Down Expand Up @@ -304,6 +304,16 @@ def test_slack_message_sanitization():
)


def test_escape_called():
with patch("re.escape") as mock_escape:
input_text = "Some text"
uids_to_remove = ["uid1"]
SlackInput._sanitize_user_message(input_text, uids_to_remove)

# Check if re.escape was called with the expected argument
mock_escape.assert_called_with("uid1")


def test_slack_init_token_parameter():
ch = SlackInput("xoxb-test", slack_signing_secret="foobar")
assert ch.slack_token == "xoxb-test"
Expand Down Expand Up @@ -415,7 +425,6 @@ def test_is_slack_message_true():


def test_is_slack_message_false():

event = {
"type": "message",
"channel": "C2147483705",
Expand Down

0 comments on commit 319811f

Please sign in to comment.