Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions misp_modules/modules/expansion/html_to_markdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,13 @@
]


def _is_ip_blocked(ip_str: str) -> bool:
def _normalize_ip_address(ip_str: str) -> ipaddress.IPv4Address | ipaddress.IPv6Address:
ip = ipaddress.ip_address(ip_str)
return ip.ipv4_mapped or ip

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle IPv4Address before reading ipv4_mapped

For any plain IPv4 literal, or for a hostname whose DNS answer includes an A record, ipaddress.ip_address() returns an IPv4Address, which does not have an ipv4_mapped attribute. This line therefore raises AttributeError before the blocklist check can return, so URLs like http://93.184.216.34/ and ordinary hostnames resolving to IPv4 addresses can crash the expansion module instead of being allowed or blocked. Guard this with getattr(ip, "ipv4_mapped", None) or an isinstance(ip, ipaddress.IPv6Address) check.

Useful? React with 👍 / 👎.



def _is_ip_blocked(ip_str: str) -> bool:
ip = _normalize_ip_address(ip_str)
return any(ip in net for net in BLOCKED_RANGES)


Expand All @@ -52,7 +57,7 @@ def _hostname_resolves_to_blocked_ip(hostname: str) -> bool:

def is_safe_url(url: str) -> bool:
parsed = urlparse(url)
if parsed.scheme not in ("http", "https"):
if parsed.scheme not in ("http", "https") or not parsed.hostname:
return False
try:
return not _is_ip_blocked(parsed.hostname)
Expand Down
40 changes: 40 additions & 0 deletions tests/test_html_to_markdown.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-

import unittest
from unittest.mock import patch

from misp_modules.modules.expansion.html_to_markdown import is_safe_url


class TestHtmlToMarkdownUrlSafety(unittest.TestCase):

def test_blocks_ipv4_mapped_ipv6_literals_for_blocked_ipv4_ranges(self):
blocked_urls = (
"http://[::ffff:127.0.0.1]/",
"http://[::ffff:10.0.0.1]/",
"http://[::ffff:172.16.0.1]/",
"http://[::ffff:192.168.0.1]/",
"http://[::ffff:169.254.169.254]/",
)

for url in blocked_urls:
with self.subTest(url=url):
self.assertFalse(is_safe_url(url))

def test_allows_public_ipv4_mapped_ipv6_literal(self):
self.assertTrue(is_safe_url("http://[::ffff:93.184.216.34]/"))

def test_blocks_hostnames_resolving_to_ipv4_mapped_blocked_addresses(self):
with patch(
"misp_modules.modules.expansion.html_to_markdown.socket.getaddrinfo",
return_value=[(None, None, None, None, ("::ffff:127.0.0.1", 0, 0, 0))],
):
self.assertFalse(is_safe_url("http://example.test/"))

def test_rejects_url_without_hostname(self):
self.assertFalse(is_safe_url("http:///missing-host"))


if __name__ == "__main__":
unittest.main()