CVE-2021-23422: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: This affects the package bikeshed before 3.0.0. This can occur when an untrusted source file containing Inline Tag Command metadata is processed. When an arbitrary OS command is executed, the command output would be included in the HTML output.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 7.8
Published: 2021-08-16T08:15Z
Repository: https://github.com/tabatkins/bikeshed
Language: Python
Files changed: bikeshed/InputSource.py, bikeshed/Spec.py, bikeshed/cli.py, bikeshed/config/main.py, bikeshed/config/retrieve.py, bikeshed/constants.py, bikeshed/extensions.py, bikeshed/inlineTags/__init__.py
Commit date: 2021-08-06 17:41:38 -0700
Commit message: Prevent escaping the source doc's folder, or running arbitrary code, without explicit opt-in at the command-line.

Diff: diff --git a/bikeshed/InputSource.py b/bikeshed/InputSource.py
index f90548427..124f1ea07 100644
--- a/bikeshed/InputSource.py
+++ b/bikeshed/InputSource.py
@@ -11,10 +11,11 @@ from typing import List, Optional
 
 import attr
 import requests
 import tenacity
 
+from . import config
 from .Line import Line
 
 
 @attr.s(auto_attribs=True)
 class InputContent:
@@ -36,21 +37,21 @@ class InputSource:
     Input can be read from stdin ("-"), an HTTPS URL, or a file. Other
     InputSources can be found relative to URLs and files, and there's a context
     manager for temporarily switching to the directory of a file InputSource.
     """
 
-    def __new__(cls, sourceName: str):
+    def __new__(cls, sourceName: str, **kwargs):
         """Dispatches to the right subclass."""
         if cls != InputSource:
             # Only take control of calls to InputSource(...) itself.
             return super().__new__(cls)
 
         if sourceName == "-":
-            return StdinInputSource(sourceName)
+            return StdinInputSource(sourceName, **kwargs)
         if sourceName.startswith("https:"):
-            return UrlInputSource(sourceName)
-        return FileInputSource(sourceName)
+            return UrlInputSource(sourceName, **kwargs)
+        return FileInputSource(sourceName, **kwargs)
 
     @abstractmethod
     def __str__(self) -> str:
         pass
 
@@ -155,15 +156,21 @@ class UrlInputSource(InputSource):
     def relative(self, relativePath) -> UrlInputSource:
         return UrlInputSource(urllib.parse.urljoin(self.sourceName, relativePath))
 
 
 class FileInputSource(InputSource):
-    def __init__(self, sourceName: str):
+    def __init__(self, sourceName: str, *, chroot: bool, chrootPath: Optional[str] = None):
         self.sourceName = sourceName
+        self.chrootPath = chrootPath
         self.type = "file"
         self.content = None
 
+        if chroot and self.chrootPath is None:
+            self.chrootPath = self.directory()
+        if self.chrootPath is not None:
+            self.sourceName = config.chrootPath(self.chrootPath, self.sourceName)
+
     def __str__(self) -> str:
         return self.sourceName
 
     def read(self) -> InputContent:
         with open(self.sourceName, encoding="utf-8") as f:
@@ -177,11 +184,11 @@ class FileInputSource(InputSource):
 
     def directory(self) -> str:
         return os.path.dirname(os.path.abspath(self.sourceName))
 
     def relative(self, relativePath) -> FileInputSource:
-        return FileInputSource(os.path.join(self.directory(), relativePath))
+        return FileInputSource(os.path.join(self.directory(), relativePath), chroot=False, chrootPath=self.chrootPath)
 
     def cheaplyExists(self, relativePath) -> bool:
         return os.access(self.relative(relativePath).sourceName, os.R_OK)
 
     def mtime(self) -> Optional[float]:
diff --git a/bikeshed/Spec.py b/bikeshed/Spec.py
index 9ac335f85..8d28c49c7 100644
--- a/bikeshed/Spec.py
+++ b/bikeshed/Spec.py
@@ -56,11 +56,11 @@ class Spec:
         if inputFilename is None:  # still
             die(
                 "No input file specified, and no *.bs or *.src.html files found in current directory.\nPlease specify an input file, or use - to pipe from STDIN."
             )
             return
-        self.inputSource = InputSource(inputFilename)
+        self.inputSource = InputSource(inputFilename, chroot=constants.chroot)
         self.transitiveDependencies = set()
         self.debug = debug
         self.token = token
         self.testing = testing
         if fileRequester is None:
diff --git a/bikeshed/cli.py b/bikeshed/cli.py
index dd4d33a92..05af94044 100644
--- a/bikeshed/cli.py
+++ b/bikeshed/cli.py
@@ -73,10 +73,22 @@ def main():
         "--die-on",
         dest="errorLevel",
         choices=["nothing", "fatal", "link-error", "warning", "everything"],
         help="Determines what sorts of errors cause Bikeshed to die (quit immediately with an error status code). Default is 'fatal'; the -f flag is a shorthand for 'nothing'",
     )
+    argparser.add_argument(
+        "--allow-nonlocal-files",
+        dest="allowNonlocalFiles",
+        action="store_true",
+        help="Allows Bikeshed to see/include files from folders higher than the one your source document is in."
+    )
+    argparser.add_argument(
+        "--allow-execute",
+        dest="allowExecute",
+        action="store_true",
+        help="Allow some features to execute arbitrary code from outside the Bikeshed codebase."
+    )
 
     subparsers = argparser.add_subparsers(title="Subcommands", dest="subparserName")
 
     specParser = subparsers.add_parser(
         "spec", help="Process a spec source file into a valid output file."
@@ -442,10 +454,12 @@ def main():
             constants.printMode = "plain"
         else:
             constants.printMode = "console"
     else:
         constants.printMode = options.printMode
+    constants.chroot = not options.allowNonlocalFiles
+    constants.executeCode = options.allowExecute
 
     update.fixupDataFiles()
     if options.subparserName == "update":
         handleUpdate(options)
     elif options.subparserName == "spec":
diff --git a/bikeshed/config/main.py b/bikeshed/config/main.py
index 92b3f7bfc..6edcd3ec5 100644
--- a/bikeshed/config/main.py
+++ b/bikeshed/config/main.py
@@ -2,10 +2,13 @@ import collections
 import os
 import re
 
 import lxml
 
+from .. import constants
+from .. import messages
+
 
 def englishFromList(items, conjunction="or"):
     # Format a list of strings into an English list.
     items = list(items)
     if len(items) == 1:
@@ -166,11 +169,23 @@ def flatten(arr):
             yield el
 
 
 def scriptPath(*pathSegs):
     startPath = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
-    return os.path.join(startPath, *pathSegs)
+    path = os.path.join(startPath, *pathSegs)
+    return path
+
+
+def chrootPath(chrootPath, path):
+    chrootPath = os.path.abspath(chrootPath)
+    path = os.path.abspath(path)
+    if not path.startswith(chrootPath):
+        messages.die(f"Attempted to access a file ({path}) outside the source document's directory ({chrootPath}). See --allow-nonlocal-files.")
+        raise Exception()
+    else:
+        return path
+
 
 
 def doEvery(s, action, lastTime=None):
     # Takes an action every N seconds.
     # Pass it the duration and the last time it took the action;
diff --git a/bikeshed/config/retrieve.py b/bikeshed/config/retrieve.py
index bfc9dee64..5c372522a 100644
--- a/bikeshed/config/retrieve.py
+++ b/bikeshed/config/retrieve.py
@@ -61,11 +61,11 @@ class DataFileRequester:
 defaultRequester = DataFileRequester(
     type="latest", fallback=DataFileRequester(type="readonly")
 )
 
 
-def retrieveBoilerplateFile(doc, name, group=None, status=None, error=True):
+def retrieveBoilerplateFile(doc, name, group=None, status=None, error=True, allowLocal=True):
     # Looks in three or four locations, in order:
     # the folder the spec source is in, the group's boilerplate folder, the megagroup's boilerplate folder, and the generic boilerplate folder.
     # In each location, it first looks for the file specialized on status, and then for the generic file.
     # Filenames must be of the format NAME.include or NAME-STATUS.include
     if group is None and doc.md.group is not None:
@@ -75,11 +75,11 @@ def retrieveBoilerplateFile(doc, name, group=None, status=None, error=True):
             status = doc.md.status
         elif doc.md.rawStatus is not None:
             status = doc.md.rawStatus
     megaGroup, status = splitStatus(status)
 
-    searchLocally = doc.md.localBoilerplate[name]
+    searchLocally = allowLocal and doc.md.localBoilerplate[name]
 
     def boilerplatePath(*segs):
         return scriptPath("boilerplate", *segs)
 
     statusFile = f"{name}-{status}.include"
@@ -99,17 +99,17 @@ def retrieveBoilerplateFile(doc, name, group=None, status=None, error=True):
                     ).format(f, name)
                 )
                 # We should remove this after giving specs time to react to the warning:
                 sources.append(doc.inputSource.relative(f))
     if group:
-        sources.append(InputSource(boilerplatePath(group, statusFile)))
-        sources.append(InputSource(boilerplatePath(group, genericFile)))
+        sources.append(InputSource(boilerplatePath(group, statusFile), chroot=False))
+        sources.append(InputSource(boilerplatePath(group, genericFile), chroot=False))
     if megaGroup:
-        sources.append(InputSource(boilerplatePath(megaGroup, statusFile)))
-        sources.append(InputSource(boilerplatePath(megaGroup, genericFile)))
-    sources.append(InputSource(boilerplatePath(statusFile)))
-    sources.append(InputSource(boilerplatePath(genericFile)))
+        sources.append(InputSource(boilerplatePath(megaGroup, statusFile), chroot=False))
+        sources.append(InputSource(boilerplatePath(megaGroup, genericFile), chroot=False))
+    sources.append(InputSource(boilerplatePath(statusFile), chroot=False))
+    sources.append(InputSource(boilerplatePath(genericFile), chroot=False))
 
     # Watch all the possible sources, not just the one that got used, because if
     # an earlier one appears, we want to rebuild.
     doc.recordDependencies(*sources)
 
diff --git a/bikeshed/constants.py b/bikeshed/constants.py
index 0d7bf5907..7290ef92a 100644
--- a/bikeshed/constants.py
+++ b/bikeshed/constants.py
@@ -7,10 +7,12 @@ quiet = True
 asciiOnly = False
 refStatus = StringEnum("current", "snapshot")
 biblioDisplay = StringEnum("index", "inline")
 specClass = None
 testAnnotationURL = "https://test.csswg.org/harness/annotate.js"
+chroot = True
+executeCode = False
 
 
 def errorLevelAt(target):
     levels = {
         "nothing": 0,
diff --git a/bikeshed/extensions.py b/bikeshed/extensions.py
index 02e4f64fc..362d6d893 100644
--- a/bikeshed/extensions.py
+++ b/bikeshed/extensions.py
@@ -1,8 +1,9 @@
 from . import config
+from . import constants
 from .h import *  # noqa: F401
 from .messages import *  # noqa: F401
 
 
 def load(doc):
-    code = config.retrieveBoilerplateFile(doc, "bs-extensions")
+    code = config.retrieveBoilerplateFile(doc, "bs-extensions", allowLocal=constants.executeCode)
     exec(code, globals())
diff --git a/bikeshed/inlineTags/__init__.py b/bikeshed/inlineTags/__init__.py
index d259b520d..999d6d8f2 100644
--- a/bikeshed/inlineTags/__init__.py
+++ b/bikeshed/inlineTags/__init__.py
@@ -1,13 +1,17 @@
 from subprocess import PIPE, Popen
 
+from .. import constants
 from ..h import *
 from ..messages import *
 
 
 def processTags(doc):
     for el in findAll("[data-span-tag]", doc):
+        if not constants.executeCode:
+            die("Found an inline code tag, but arbitrary code execution isn't allowed. See the --allow-execute flag.")
+            return
         tag = el.get("data-span-tag")
         if tag not in doc.md.inlineTagCommands:
             die("Unknown inline tag '{0}' found:\n  {1}", tag, outerHTML(el), el=el)
             continue
         command = doc.md.inlineTagCommands[tag]

Vulnerable code:     def __new__(cls, sourceName: str):
            return StdinInputSource(sourceName)
            return UrlInputSource(sourceName)
        return FileInputSource(sourceName)
    def __init__(self, sourceName: str):
        return FileInputSource(os.path.join(self.directory(), relativePath))
        self.inputSource = InputSource(inputFilename)
    return os.path.join(startPath, *pathSegs)
def retrieveBoilerplateFile(doc, name, group=None, status=None, error=True):
    searchLocally = doc.md.localBoilerplate[name]
        sources.append(InputSource(boilerplatePath(group, statusFile)))
        sources.append(InputSource(boilerplatePath(group, genericFile)))
        sources.append(InputSource(boilerplatePath(megaGroup, statusFile)))
        sources.append(InputSource(boilerplatePath(megaGroup, genericFile)))
    sources.append(InputSource(boilerplatePath(statusFile)))
    sources.append(InputSource(boilerplatePath(genericFile)))
    code = config.retrieveBoilerplateFile(doc, "bs-extensions")
Fixed code: from . import config
    def __new__(cls, sourceName: str, **kwargs):
            return StdinInputSource(sourceName, **kwargs)
            return UrlInputSource(sourceName, **kwargs)
        return FileInputSource(sourceName, **kwargs)
    def __init__(self, sourceName: str, *, chroot: bool, chrootPath: Optional[str] = None):
        self.chrootPath = chrootPath
        if chroot and self.chrootPath is None:
            self.chrootPath = self.directory()
        if self.chrootPath is not None:
            self.sourceName = config.chrootPath(self.chrootPath, self.sourceName)

        return FileInputSource(os.path.join(self.directory(), relativePath), chroot=False, chrootPath=self.chrootPath)
        self.inputSource = InputSource(inputFilename, chroot=constants.chroot)
    argparser.add_argument(
        "--allow-nonlocal-files",
        dest="allowNonlocalFiles",
        action="store_true",
        help="Allows Bikeshed to see/include files from folders higher than the one your source document is in."
    )
    argparser.add_argument(
        "--allow-execute",
        dest="allowExecute",
        action="store_true",
        help="Allow some features to execute arbitrary code from outside the Bikeshed codebase."
    )
    constants.chroot = not options.allowNonlocalFiles
    constants.executeCode = options.allowExecute
from .. import constants
from .. import messages

    path = os.path.join(startPath, *pathSegs)
    return path


def chrootPath(chrootPath, path):
    chrootPath = os.path.abspath(chrootPath)
    path = os.path.abspath(path)
    if not path.startswith(chrootPath):
        messages.die(f"Attempted to access a file ({path}) outside the source document's directory ({chrootPath}). See --allow-nonlocal-files.")
        raise Exception()
    else:
        return path

def retrieveBoilerplateFile(doc, name, group=None, status=None, error=True, allowLocal=True):
    searchLocally = allowLocal and doc.md.localBoilerplate[name]
        sources.append(InputSource(boilerplatePath(group, statusFile), chroot=False))
        sources.append(InputSource(boilerplatePath(group, genericFile), chroot=False))
        sources.append(InputSource(boilerplatePath(megaGroup, statusFile), chroot=False))
        sources.append(InputSource(boilerplatePath(megaGroup, genericFile), chroot=False))
    sources.append(InputSource(boilerplatePath(statusFile), chroot=False))
    sources.append(InputSource(boilerplatePath(genericFile), chroot=False))
chroot = True
executeCode = False
from . import constants
    code = config.retrieveBoilerplateFile(doc, "bs-extensions", allowLocal=constants.executeCode)
from .. import constants
        if not constants.executeCode:
            die("Found an inline code tag, but arbitrary code execution isn't allowed. See the --allow-execute flag.")
            return