diff options
| author | Eric Biggers <ebiggers@google.com> | 2022-02-23 12:35:04 -0800 |
|---|---|---|
| committer | Eric Biggers <ebiggers@google.com> | 2022-02-23 12:35:04 -0800 |
| commit | 1a47718420317f893831b0223153d56005d5b02b (patch) | |
| tree | b5cdbe761bee21efc4d643083c4bc76065d132fd /filesystem/filesystem_test.go | |
| parent | fa1a1fdbdea65829ce24a6b6f86ce2961e465b02 (diff) | |
filesystem: validate size and type of metadata files
Don't allow reading metadata files that are very large, as they can
crash the program due to the memory required. Similarly, don't allow
reading metadata files that aren't regular files, such as FIFOs, or
symlinks (which could point to a device node like /dev/zero), as that
can hang the program. Both issues were particularly problematic for
pam_fscrypt, as they could prevent users from being able to log in.
Note: these checks are arguably unneeded if we strictly check the file
ownership too, which a later commit will do. But there's no reason not
to do these basic checks too.
Diffstat (limited to 'filesystem/filesystem_test.go')
| -rw-r--r-- | filesystem/filesystem_test.go | 63 |
1 files changed, 63 insertions, 0 deletions
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go index 92726b2..71724ae 100644 --- a/filesystem/filesystem_test.go +++ b/filesystem/filesystem_test.go @@ -24,9 +24,11 @@ import ( "log" "os" "path/filepath" + "syscall" "testing" "github.com/golang/protobuf/proto" + "golang.org/x/sys/unix" "github.com/google/fscrypt/crypto" "github.com/google/fscrypt/metadata" @@ -382,3 +384,64 @@ func TestLinkedProtector(t *testing.T) { t.Errorf("protector %+v does not equal expected protector %+v", retProtector, protector) } } + +func createFile(path string, size int64) error { + if err := ioutil.WriteFile(path, []byte{}, 0600); err != nil { + return err + } + return os.Truncate(path, size) +} + +// Tests the readMetadataFileSafe() function. +func TestReadMetadataFileSafe(t *testing.T) { + tempDir, err := ioutil.TempDir("", "fscrypt") + if err != nil { + t.Fatal(err) + } + filePath := filepath.Join(tempDir, "file") + defer os.RemoveAll(tempDir) + + // Good file (control case) + if err = createFile(filePath, 1000); err != nil { + t.Fatal(err) + } + if _, err = readMetadataFileSafe(filePath); err != nil { + t.Fatal("failed to read file") + } + os.Remove(filePath) + + // Nonexistent file + _, err = readMetadataFileSafe(filePath) + if !os.IsNotExist(err) { + t.Fatal("trying to read nonexistent file didn't fail with expected error") + } + + // Symlink + if err = os.Symlink("target", filePath); err != nil { + t.Fatal(err) + } + _, err = readMetadataFileSafe(filePath) + if err.(*os.PathError).Err != syscall.ELOOP { + t.Fatal("trying to read symlink didn't fail with ELOOP") + } + os.Remove(filePath) + + // FIFO + if err = unix.Mkfifo(filePath, 0600); err != nil { + t.Fatal(err) + } + _, err = readMetadataFileSafe(filePath) + if _, ok := err.(*ErrCorruptMetadata); !ok { + t.Fatal("trying to read FIFO didn't fail with expected error") + } + os.Remove(filePath) + + // Very large file + if err = createFile(filePath, 1000000); err != nil { + t.Fatal(err) + } + _, err = readMetadataFileSafe(filePath) + if _, ok := err.(*ErrCorruptMetadata); !ok { + t.Fatal("trying to read very large file didn't fail with expected error") + } +} |