The script detect7.sh introduces security vulnerabilities

[AlephNotation]

Hi there.

I was recently reviewing the repo and noticed the addition of a new script: detect7.sh. I looked it over and noticed the following issues I would like to raise.

(1) A script like this should not be part of any public facing repository because it spells out in detail the types of security scans RTX is running. Something like this probably belongs in the CI/CD apparatus itself, not the repo being scanned. I am by no means a fan of 'security via obscurity', but publishing code that informs people of the exact attack vectors RTX is monitoring (and thus how to avoid them) seems like a practice that merits discussion.

(2) The above referenced script seems to be using hardcoded passwords. Examine the following code block:

for i in $*; do
  if [[ $i == --blackduck.hub.password=* ]]; then
    LOGGABLE_SCRIPT_ARGS="$LOGGABLE_SCRIPT_ARGS --blackduck.hub.password=<redacted>"
  elif [[ $i == --blackduck.hub.proxy.password=* ]]; then
    LOGGABLE_SCRIPT_ARGS="$LOGGABLE_SCRIPT_ARGS --blackduck.hub.proxy.password=<redacted>"
  elif [[ $i == --blackduck.hub.api.token=* ]]; then
    LOGGABLE_SCRIPT_ARGS="$LOGGABLE_SCRIPT_ARGS --blackduck.hub.api.token=<redacted>"
  elif [[ $i == --blackduck.password=* ]]; then
    LOGGABLE_SCRIPT_ARGS="$LOGGABLE_SCRIPT_ARGS --blackduck.password=<redacted>"
  elif [[ $i == --blackduck.proxy.password=* ]]; then
    LOGGABLE_SCRIPT_ARGS="$LOGGABLE_SCRIPT_ARGS --blackduck.proxy.password=<redacted>"
  elif [[ $i == --blackduck.api.token=* ]]; then
    LOGGABLE_SCRIPT_ARGS="$LOGGABLE_SCRIPT_ARGS --blackduck.api.token=<redacted>"
  elif [[ $i == --polaris.access.token=* ]]; then
    LOGGABLE_SCRIPT_ARGS="$LOGGABLE_SCRIPT_ARGS --polaris.access.token=<redacted>"
  else
    LOGGABLE_SCRIPT_ARGS="$LOGGABLE_SCRIPT_ARGS $i"
  fi
done

Passwords should under no circumstances be handled as inline plaintext and this doubly applies to code that is committed to source.

(3). Examine the following code block:

get_detect() {
  PATH_SEPARATOR=$(get_path_separator)
  USE_LOCAL=0
  LOCAL_FILE="${DETECT_JAR_DOWNLOAD_DIR}${PATH_SEPARATOR}synopsys-detect-last-downloaded-jar.txt"
  if [[ -z "${DETECT_SOURCE}" ]]; then
    if [[ -z "${DETECT_RELEASE_VERSION}" ]]; then
      VERSION_CURL_CMD="curl ${DETECT_CURL_OPTS} --silent --header \"X-Result-Detail: info\" '${DETECT_BINARY_REPO_URL}/api/storage/bds-integrations-release/com/synopsys/integration/synopsys-detect?properties=${DETECT_VERSION_KEY}'"
      VERSION_EXTRACT_CMD="${VERSION_CURL_CMD} | grep \"${DETECT_VERSION_KEY}\" | sed 's/[^[]*[^\"]*\"\([^\"]*\).*/\1/'"
      DETECT_SOURCE=$(eval ${VERSION_EXTRACT_CMD})
      if [[ -z "${DETECT_SOURCE}" ]]; then
        echo "Unable to derive the location of ${DETECT_VERSION_KEY} from response to: ${VERSION_CURL_CMD}"
        USE_LOCAL=1
      fi
    else
      DETECT_SOURCE="${DETECT_BINARY_REPO_URL}/bds-integrations-release/com/synopsys/integration/synopsys-detect/${DETECT_RELEASE_VERSION}/synopsys-detect-${DETECT_RELEASE_VERSION}.jar"
    fi
  fi

Downloading and running unknown jars without verification is a security vulnerability itself. While validating against a checksum does not itself mitigate against this added attack vector (supply chain attacks are a thing), it does at least allow the user running this script to verify the origin of their download.

Furthermore, the recently uncovered Log4j creates additional pause as there is no way for the user to verify that the downloaded jars do not themselves introduce further critical security vulnerabilities.

[lukeutc]

For the detect file, I've deleted from the repo. Is there a way to remove it from the repo history?

[AlephNotation]

No.

[bethfrenkel]

@lukeutc https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History

[AlephNotation]

Ok I should actually explain here.

It is possible to rewrite the commit history. However I'm not sure that matters anymore. This project has been forked and there is no way to change the commit history of those forks. Additionally, this repo has already been indexed by at least one public third party tracker and I expect there are probably more private indexers that have already cached it.