Crash occurs when doing recursive scan against a directory that is denied access by AppArmor

[brlin-tw]

Steps to reproduce

1. Launch an Ubuntu VM (I use this Vagrant-based VM implementation).

2. Install vt to /usr/local/bin as root.

3. Setting up an API key.

4. Run the following command to create a directory to be denied by AppArmor:

   mkdir -p ~/denied-dir

5. Create the /etc/apparmor.d/usr.local.bin.vt Apparmor profile file as root with the following content:

   abi <abi/3.0>,
   
   include <tunables/global>
   
   /usr/local/bin/vt {
   include <abstractions/base>
   include <abstractions/nameservice>
   include <abstractions/ssl_certs>
   
   /home/*/** r,
   deny owner /home/*/denied-dir/ r,
   
   /etc/hosts r,
   /etc/nsswitch.conf r,
   /run/systemd/resolve/stub-resolv.conf r,
   /usr/local/bin/vt mr,
   owner /home/*/.cache/.vt.relationships.cache rw,
   owner /home/*/.vt.toml rw,
   }
   

6. Run the following command as root to load the AppArmor profile in enforcing mode:

   apparmor_parser -r /etc/apparmor.d/usr.local.bin.vt

7. Run the following command to trigger the crash:

   vt scan file -r ~/denied-dir

Current behavior

The program crashed with:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x82be08]

goroutine 7 [running]:
github.com/VirusTotal/vt-cli/utils.(*StringArrayReader).ReadString(0x0?)
	/home/runner/work/vt-cli/vt-cli/utils/string_reader.go:46 +0x8
github.com/VirusTotal/vt-cli/utils.(*Coordinator).DoWithStringsFromReader.func1()
	/home/runner/work/vt-cli/vt-cli/utils/do.go:69 +0x2f
created by github.com/VirusTotal/vt-cli/utils.(*Coordinator).DoWithStringsFromReader in goroutine 1
	/home/runner/work/vt-cli/vt-cli/utils/do.go:68 +0x95

Expected behavior

No crash

Version

1.3.0

[plusvic]

@JicLotus

[JicLotus]

Hi @brlin-tw. Thanks for sharing the details. I’ll take a closer look in the next couple of days, if that timeline works for you.

In the meantime, beyond addressing the crash for this scenario, could you clarify the expected behavior? e.g. should the system simply return a “permission denied” error for the affected directory or subdirectories, or should it continue scanning other accessible paths while skipping those without sufficient permissions?

I want to align with the intended behavior as well.

Best regards

cc @plusvic

[plusvic]

Hi @brlin-tw. Thanks for sharing the details. I’ll take a closer look in the next couple of days, if that timeline works for you.

In the meantime, beyond addressing the crash for this scenario, could you clarify the expected behavior? e.g. should the system simply return a “permission denied” error for the affected directory or subdirectories, or should it continue scanning other accessible paths while skipping those without sufficient permissions?

I want to align with the intended behavior as well.

Best regards

cc @plusvic

I would say that scanning should continue after finding an error, otherwise scanning large directory that contains non-accessible path could be painful, and you should start over again all the time.

[brlin-tw]

@JicLotus

I’ll take a closer look in the next couple of days, if that timeline works for you.

I just unintentionally triggered this issue so that's fine by me.

In the meantime, beyond addressing the crash for this scenario, could you clarify the expected behavior? e.g. should the system simply return a “permission denied” error for the affected directory or subdirectories, or should it continue scanning other accessible paths while skipping those without sufficient permissions?

I agree with @plusvic here. The command should emit an access denied error but still proceed in scanning rest of the accessible files.

[JicLotus]

Hello @plusvic @brlin-tw

I would say that scanning should continue after finding an error, otherwise scanning large directory that contains non-accessible path could be painful, and you should start over again all the time.
The command should emit an access denied error but still proceed in scanning rest of the accessible files.

Totally. That aligns with my thinking as well, I just wanted to confirm we were on the same page. Thanks to you both.

---

Apologies for the delayed response. I’ve been tied up with a few other compromises, but I'll aim to have a solution ready over the weekend.

[JicLotus]

Hello @brlin-tw, Hope you’re doing well! I’ve been reviewing your error stack and wanted to clarify a couple of things:

1. Is this issue occurring in version 1.3.0 without the -r flag as well?
2. Could you try downgrading to version 1.2.0 (which doesn’t support the -r flag) and see if the issue persists?

I'm asking to rule out any recent changes or interactions with the -r flag. Based on the nature of the error, I ran quick tests with both scenarios above(#1 and #2), and the issue still occurs. It appears to be related to a nil reference when an error is returned, which suggests this might be an older issue.

A quick fix is to always return the elements, even if there’s an accompanying error.

---

In any case, this is the fix #129

cc: @plusvic

[brlin-tw]

1. Is this issue occurring in version 1.3.0 without the -r flag as well?

Yes:

$ vt scan file ~/denied-dir
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x82b9c8]

goroutine 20 [running]:
github.com/VirusTotal/vt-cli/utils.(*StringArrayReader).ReadString(0x0?)
        /build/vt-cli/parts/vt-cli/build/utils/string_reader.go:46 +0x8
github.com/VirusTotal/vt-cli/utils.(*Coordinator).DoWithStringsFromReader.func1()
        /build/vt-cli/parts/vt-cli/build/utils/do.go:69 +0x2f
created by github.com/VirusTotal/vt-cli/utils.(*Coordinator).DoWithStringsFromReader in goroutine 1
        /build/vt-cli/parts/vt-cli/build/utils/do.go:68 +0x95

2. Could you try downgrading to version 1.2.0 (which doesn’t support the -r flag) and see if the issue persists?

This issue is reproduced on the 1.2.0 version as well:

$ vt scan file ~/denied-dir
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x82e4a8]

goroutine 2 [running]:
github.com/VirusTotal/vt-cli/utils.(*StringArrayReader).ReadString(0x1?)
        /build/vt-cli/parts/vt-cli/build/utils/string_reader.go:46 +0x8
github.com/VirusTotal/vt-cli/utils.(*Coordinator).DoWithStringsFromReader.func1()
        /build/vt-cli/parts/vt-cli/build/utils/do.go:69 +0x2f
created by github.com/VirusTotal/vt-cli/utils.(*Coordinator).DoWithStringsFromReader in goroutine 1
        /build/vt-cli/parts/vt-cli/build/utils/do.go:68 +0x95