Repository: DataDog/guarddog
Pull request #431 (2024-07-31 14:00:18)

Improve shady-links rule
Improving patterns in shady links:
* Standardising Allowed chars in the domain 
* Added some interesting domains from https://lots-project.com/ (Live of trusted Sites)
* Excluded some FP


Patch:
diff --git a/guarddog/analyzer/sourcecode/shady-links.yml b/guarddog/analyzer/sourcecode/shady-links.yml
index 8bab71e..c2c667b 100644
--- a/guarddog/analyzer/sourcecode/shady-links.yml
+++ b/guarddog/analyzer/sourcecode/shady-links.yml
@@ -5,10 +5,9 @@ rules:
     metadata:
       description: Identify when a package contains an URL to a domain with a suspicious extension
     patterns:
-      # Semgrep not robust enough to ignore comments in lists
-      - pattern-not-regex: \# .*
 
       # ignore comments
+      - pattern-not-regex: ^\s*\# .*
       - pattern-not-regex: ^\s*\/\*(.|\n)*?\*\/\s*$
       - pattern-not-regex: ^\s*\/\/.*$
 
@@ -16,19 +15,22 @@ rules:
       - pattern-not-regex: ^\s*"""(.|\n)*?"""\s*$
 
       # Exclude local IPv4 sometimes used in tests
-      - pattern-not-regex: (http[s]?:\/\/[^/?#]*(?:192\.168|10\.\d{1,3}|172\.(?:1[6-9]|2\d|3[0-1])|127\.\d{1,3})\.\d{1,3}\.\d{1,3}|0\.0\.0\.0|localhost)
+      - pattern-not-regex: (http[s]?:\/\/[^\n\[\/\?#"']*?(?:192\.168|10\.\d{1,3}|172\.(?:1[6-9]|2\d|3[0-1])|127\.\d{1,3})\.\d{1,3}\.\d{1,3}|0\.0\.0\.0|localhost)
 
       # Exclude public IPv4 sometimes used in tests
-      - pattern-not-regex: (http[s]?:\/\/[^/?#]*(?:1\.1\.1\.1|8\.8\.8\.8))
+      - pattern-not-regex: (http[s]?:\/\/[^\n\[\/\?#"']*?(?:1\.1\.1\.1|8\.8\.8\.8))
 
       - patterns:
         - pattern: ("...")
         - pattern-either:
-            - pattern-regex: (http[s]?:\/\/bit\.ly.*)$
-            - pattern-regex: (http[s]?:\/\/.*\.(link|xyz|tk|ml|ga|cf|gq|pw|top|club|mw|bd|ke|am|sbs|date|quest|cd|bid|cd|ws|icu|cam|uno|email|stream))$
-            - pattern-regex: (http[s]?:\/\/.*\.(link|xyz|tk|ml|ga|cf|gq|pw|top|club|mw|bd|ke|am|sbs|date|quest|cd|bid|cd|ws|icu|cam|uno|email|stream)\/)
-            - pattern-regex: (http[s]?:\/\/[^/?#]*(?:\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}))
-            - pattern-regex: (http[s]?:\/\/[^\n\[/?#]*?(?:\[(([A-Fa-f0-9]{1,4}:){0,7}|:):?[A-Fa-f0-9]{1,4}(:[A-Fa-f0-9]{1,4}){0,7})\])
+            # complete domains
+            - pattern-regex: (http[s]?:\/\/[^\n\[\/\?#"']*?(bit\.ly|discord\.com|workers\.dev|transfer\.sh|filetransfer\.io|sendspace\.com|appdomain\.cloud|backblazeb2\.com\|paste\.ee|ngrok\.io|termbin\.com|localhost\.run|webhook\.site|oastify\.com|burpcollaborator\.me)\/)
+            # top-level domains
+            - pattern-regex: (http[s]?:\/\/[^\n\[\/\?#"']*?\.(link|xyz|tk|ml|ga|cf|gq|pw|top|club|mw|bd|ke|am|sbs|date|quest|cd|bid|cd|ws|icu|cam|uno|email|stream)\/)
+            # IPv4
+            - pattern-regex: (http[s]?:\/\/[^\n\[\/\?#"']*?(?:\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}))
+            # IPv6
+            - pattern-regex: (http[s]?:\/\/[^\n\[\/\?#"']*?(?:\[(([A-Fa-f0-9]{1,4}:){0,7}|:):?[A-Fa-f0-9]{1,4}(:[A-Fa-f0-9]{1,4}){0,7})\])
     paths:
       exclude:
         - "*/test/*"
diff --git a/guarddog/utils/archives.py b/guarddog/utils/archives.py
index 586fb46..4c278d6 100644
--- a/guarddog/utils/archives.py
+++ b/guarddog/utils/archives.py
@@ -1,8 +1,7 @@
 import logging
 import os
-import stat
 import zipfile
-
+import stat
 import tarsafe  # type:ignore
 
 log = logging.getLogger("guarddog")
@@ -10,25 +9,41 @@ log = logging.getLogger("guarddog")
 
 def is_supported_archive(path: str) -> bool:
     """
-    Decide whether a file contains a supported archive based on its
-    file extension.
+    Decide whether a file contains a supported archive.
 
     Args:
         path (str): The local filesystem path to examine
 
     Returns:
         bool: Represents the decision reached for the file
+    """
+    return is_tar_archive(path) or is_zip_archive(path)
+
 
+def is_tar_archive(path: str) -> bool:
     """
-    def is_tar_archive(path: str) -> bool:
-        tar_exts = [".bz2", ".bzip2", ".gz", ".gzip", ".tgz", ".xz"]
+    Decide whether a file contains a tar archive.
 
-        return any(path.endswith(ext) for ext in tar_exts)
+    Args:
+        path (str): The local filesystem path to examine
 
-    def is_zip_archive(path: str) -> bool:
-        return any(path.endswith(ext) for ext in [".zip", ".whl", ".egg"])
+    Returns:
+        bool: Represents the decision reached for the file
+    """
+    return any(path.endswith(ext) for ext in [".tar.gz", ".tgz"])
 
-    return is_tar_archive(path) or is_zip_archive(path)
+
+def is_zip_archive(path: str) -> bool:
+    """
+    Decide whether a file contains a zip, whl or egg archive.
+
+    Args:
+        path (str): The local filesystem path to examine
+
+    Returns:
+        bool: Represents the decision reached for the file
+    """
+    return any(path.endswith(ext) for ext in [".zip", ".whl", ".egg"])
 
 
 def safe_extract(source_archive: str, target_directory: str) -> None:
@@ -46,7 +61,7 @@ def safe_extract(source_archive: str, target_directory: str) -> None:
 
     """
     log.debug(f"Extracting archive {source_archive} to directory {target_directory}")
-    if tarsafe.is_tarfile(source_archive):
+    if is_tar_archive(source_archive):
 
         def add_exec(path):
             st = os.stat(path)
@@ -67,7 +82,7 @@ def safe_extract(source_archive: str, target_directory: str) -> None:
         tarsafe.open(source_archive).extractall(target_directory)
         recurse_add_perms(target_directory)
 
-    elif zipfile.is_zipfile(source_archive):
+    elif is_zip_archive(source_archive):
         with zipfile.ZipFile(source_archive, 'r') as zip:
             for file in zip.namelist():
                 # Note: zip.extract cleans up any malicious file name


Test patch:
diff --git a/tests/analyzer/sourcecode/shady-links.py b/tests/analyzer/sourcecode/shady-links.py
index 9d3486d..b13860b 100644
--- a/tests/analyzer/sourcecode/shady-links.py
+++ b/tests/analyzer/sourcecode/shady-links.py
@@ -149,3 +149,23 @@ def f():
       data=json.dumps(data).encode("utf-8", errors="ignore"),
       headers=headers,
   )
+
+  def f():
+    auth_config = KubernetesUserPasswordConfig(
+                username=kube_config.username,
+                password=kube_config.password,
+                server=kube_config.host,
+                certificate_authority=base64.urlsafe_b64encode(
+                    open(kube_config.ssl_ca_cert, "rb").read()
+                ).decode("utf-8")
+                if kube_config.ssl_ca_cert
+                else None,
+                # ok: shady-links
+                cluster_name=kube_config.host.strip("https://").split(":")[0],
+                insecure=kube_config.verify_ssl is False,
+            )
+
+  def f():
+    # ruleid: shady-links
+    trackingServiceUrl = 'https://b.alt-h7-eoj8gqk1.workers.dev/track'
+
diff --git a/tests/core/test_cli.py b/tests/core/test_cli.py
index 094c84d..8f0eed1 100644
--- a/tests/core/test_cli.py
+++ b/tests/core/test_cli.py
@@ -1,8 +1,5 @@
 import unittest
 import unittest.mock as mock
-import zipfile
-
-import tarsafe
 
 import guarddog.cli
 from guarddog.ecosystems import ECOSYSTEM
@@ -102,10 +99,10 @@ class TestCli(unittest.TestCase):
                 isfile.return_value = True
                 # The next two patches are to make sure we don't try
                 # to extract the test filename
-                with mock.patch("tarsafe.is_tarfile") as is_tar:
-                    is_tar.return_value = False
-                    with mock.patch("zipfile.is_zipfile") as is_zip:
-                        is_zip.return_value = False
+                with mock.patch("guarddog.utils.archives.is_tar_archive") as istar:
+                    istar.return_value = False
+                    with mock.patch("guarddog.utils.archives.is_zip_archive") as iszip:
+                        iszip.return_value = False
                         with mock.patch.object(scanner.PackageScanner, 'scan_local', return_value={}) as _:
                             try:
                                 with self.assertLogs("guarddog", level="DEBUG") as cm: