Misconfiguration: False positive when using ADD with a URL in Dockerfiles

[nicwortel]

IDs

AVD-DS-0005

Description

The ADD instead of COPY check reports Dockerfiles which use ADD where COPY could have been used.

However, it also reports ADD commands which download a URL, which is not supported by the COPY command.

I believe this is a false positive, and the check should not report ADD commands which download a URL.

The ADD instruction is best for when you need to download a remote artifact as part of your build. ADD is better than manually adding files using something like wget and tar, because it ensures a more precise build cache. ADD also has built-in support for checksum validation of the remote resources, and a protocol for parsing branches, tags, and subdirectories from Git URLs.

Source: https://docs.docker.com/build/building/best-practices/#add-or-copy

The check already rules out uses of the ADD command where the arguments contain .tar. I believe this should be extended with checking whether the arguments contain http://, https:// or git@, and in that case not report the failure:

get_add[output] {
	add := docker.add[_]
	args := concat(" ", add.Value)

	not contains(args, ".tar")
+	not contains(args, "http://")
+	not contains(args, "https://")
+	not contains(args, "git@")

	not is_command_with_hash(add.Value, "file:")
	not is_command_with_hash(add.Value, "multi:")

	output := {
		"args": args,
		"cmd": add,
	}
}

If someone can verify that this is the correct approach, I'd be happy to submit a pull request.

Reproduction Steps

1. Have a Dockerfile with an ADD command which downloads a file from a URL:

FROM scratch

ADD https://example.com/archive.zip /usr/src/things/

2. Run trivy config Dockerfile
3. Notice that Trivy reports a failure on line 3 (the ADD command)

Target

Filesystem

Scanner

Misconfiguration

Target OS

No response

Debug Output

2024-10-24T14:55:39+02:00	DEBUG	Parsed severities	severities=[UNKNOWN LOW MEDIUM HIGH CRITICAL]
2024-10-24T14:55:39+02:00	DEBUG	Cache dir	dir="/home/nic/.cache/trivy"
2024-10-24T14:55:39+02:00	INFO	Misconfiguration scanning is enabled
2024-10-24T14:55:39+02:00	DEBUG	Policies successfully loaded from disk
2024-10-24T14:55:39+02:00	DEBUG	Enabling misconfiguration scanners	scanners=[azure-arm cloudformation dockerfile helm kubernetes terraform terraformplan-json terraformplan-snapshot]
2024-10-24T14:55:40+02:00	DEBUG	[nuget] The nuget packages directory couldn't be found. License search disabled
2024-10-24T14:55:40+02:00	DEBUG	Scanning files for misconfigurations...	scanner="Dockerfile"
2024-10-24T14:55:40+02:00	DEBUG	[misconf] 55:40.039873219 dockerfile.scanner.rego          Overriding filesystem for checks!
2024-10-24T14:55:40+02:00	DEBUG	[misconf] 55:40.041109357 dockerfile.scanner.rego          Loaded 3 embedded libraries.
2024-10-24T14:55:40+02:00	DEBUG	[misconf] 55:40.118882547 dockerfile.scanner.rego          Loaded 191 embedded policies.
2024-10-24T14:55:40+02:00	DEBUG	[misconf] 55:40.218438536 dockerfile.scanner.rego          Loaded 195 policies from disk.
2024-10-24T14:55:40+02:00	DEBUG	[misconf] 55:40.219410744 dockerfile.scanner.rego          Overriding filesystem for data!
2024-10-24T14:55:40+02:00	DEBUG	[misconf] 55:40.677246483 dockerfile.scanner.rego          Scanning 1 inputs...
2024-10-24T14:55:40+02:00	DEBUG	OS is not detected.
2024-10-24T14:55:40+02:00	INFO	Detected config files	num=1
2024-10-24T14:55:40+02:00	DEBUG	Scanned config file	path="Dockerfile"

Dockerfile (dockerfile)

Tests: 27 (SUCCESSES: 24, FAILURES: 3, EXCEPTIONS: 0)
Failures: 3 (UNKNOWN: 0, LOW: 2, MEDIUM: 0, HIGH: 1, CRITICAL: 0)

HIGH: Specify at least 1 USER command in Dockerfile with non-root user as argument
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Running containers with 'root' user can lead to a container escape situation. It is a best practice to run containers as non-root users, which can be done by adding a 'USER' statement to the Dockerfile.

See https://avd.aquasec.com/misconfig/ds002
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────


LOW: Consider using 'COPY https://example.com/archive.zip /usr/src/things/' command instead of 'ADD https://example.com/archive.zip /usr/src/things/'
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
You should use COPY instead of ADD unless you want to extract a tar file. Note that an ADD command will extract a tar file, which adds the risk of Zip-based vulnerabilities. Accordingly, it is advised to use a COPY command, which does not extract tar files.

See https://avd.aquasec.com/misconfig/ds005
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 Dockerfile:3
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   3 [ ADD https://example.com/archive.zip /usr/src/things/
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────


LOW: Add HEALTHCHECK instruction in your Dockerfile
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
You should add HEALTHCHECK instruction in your docker container images to perform the health check on running containers.

See https://avd.aquasec.com/misconfig/ds026
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Version

Version: 0.51.1
Vulnerability DB:
  Version: 2
  UpdatedAt: 2024-10-23 12:17:15.932246987 +0000 UTC
  NextUpdate: 2024-10-24 12:17:15.932246847 +0000 UTC
  DownloadedAt: 2024-10-23 15:31:17.952774652 +0000 UTC
Java DB:
  Version: 1
  UpdatedAt: 2024-09-14 01:10:23.037590517 +0000 UTC
  NextUpdate: 2024-09-17 01:10:23.037590387 +0000 UTC
  DownloadedAt: 2024-09-14 14:05:35.064365435 +0000 UTC
Check Bundle:
  Digest: sha256:ef2d9ad4fce0f933b20a662004d7e55bf200987c180e7f2cd531af631f408bb3
  DownloadedAt: 2024-10-23 14:40:05.798176344 +0000 UTC

Checklist

• Read the documentation regarding wrong detection
• Ran Trivy with -f json that shows data sources and confirmed that the security advisory in data sources was correct

[nicwortel]

As a side note, there might be more cases where using ADD instead of COPY should be allowed, such as filenames containing .zip or other archive extensions.

[nikpivkin]

Hi @nicwortel !

I think you're right. We should improve this check so that it doesn't trigger for ADD specific sources:

• If is a local tar archive, it is decompressed and extracted to the specified destination
• If is a URL, the contents of the URL are downloaded and placed at the specified destination
• If is a Git repository, the repository is cloned to the specified destination

/cc @simar7

[nikpivkin]

Docker detects the archive by the file header, but we don't have access to the files, so we have to rely only on the extensions.

[nikpivkin]

https://github.com/moby/moby/blob/master/builder/remotecontext/urlutil/urlutil.go#L80-L82

[nicwortel]

@nikpivkin do you agree with my proposed change? Shall I submit a pull request to the https://github.com/aquasecurity/trivy-checks repository?

[nikpivkin]

@nicwortel Yes, contributions are welcome

[simar7]

@nicwortel we welcome your PR, I've opened #7806 to track it.