fix(elf)!: return errors from IsElfFile (#5296)

IsElfFile silently swallowed open/read errors, making it impossible
for callers to distinguish "not ELF" from "could not check." Change
the signature to (bool, error) so callers can act on permission,
I/O, or missing-file failures.

- Use io.ReadFull to correctly handle empty and short files as
  (false, nil) instead of leaking io.EOF as an error.
- Extract hasElfMagic([4]byte) so callers with a known-size buffer
  skip the redundant length guard; HasElfMagic([]byte) delegates
  to it after the bounds check.
- Replace opaque decimal magic constants with 0x7F,'E','L','F'.
- Add tests for permission-denied, short-file, and empty-file
  error contract.

BREAKING CHANGE: IsElfFile returns (bool, error) instead of bool.

Author: geyslan

common/elf/analyzer.go

@@ -96,15 +96,20 @@ func Is32BitMachine(machine elf.Machine) bool {
 	}
 }
 
+func hasElfMagic(magic [4]byte) bool {
+	return magic[0] == 0x7F &&
+		magic[1] == 'E' &&
+		magic[2] == 'L' &&
+		magic[3] == 'F'
+}
+
 // HasElfMagic checks if the given bytes start with the ELF magic number (0x7F 'ELF').
 // This is a fast check that only validates the first 4 bytes.
 func HasElfMagic(bytesArray []byte) bool {
-	if len(bytesArray) >= 4 {
-		if bytesArray[0] == 127 && bytesArray[1] == 69 && bytesArray[2] == 76 && bytesArray[3] == 70 {
-			return true
-		}
+	if len(bytesArray) < 4 {
+		return false
 	}
-	return false
+	return hasElfMagic([4]byte(bytesArray[:4]))
 }
 
 // IsElf checks if the given bytes represent a valid ELF file.
@@ -114,11 +119,13 @@ func IsElf(bytesArray []byte) bool {
 	return HasElfMagic(bytesArray)
 }
 
-// IsElfFile checks if the file at the given path is an ELF file (fast magic-only check).
-func IsElfFile(filePath string) bool {
+// IsElfFile performs a cheap 4-byte magic check on the file at path.
+// It returns (true, nil) for ELF files, (false, nil) for non-ELF files that
+// were readable, and (false, err) when the file could not be opened or read.
+func IsElfFile(filePath string) (bool, error) {
 	file, err := os.Open(filePath)
 	if err != nil {
-		return false
+		return false, err
 	}
 	// Suppress the default readahead window (commonly 128 KiB, see /sys/block/*/queue/read_ahead_kb)
 	// since we only read the 4-byte ELF magic; this avoids pulling unneeded pages into the
@@ -132,13 +139,16 @@ func IsElfFile(filePath string) bool {
 		}
 	}()
 
-	magic := make([]byte, 4)
-	n, err := file.Read(magic)
-	if err != nil || n < 4 {
-		return false
+	var magic [4]byte
+	_, err = io.ReadFull(file, magic[:])
+	if err == io.EOF || err == io.ErrUnexpectedEOF {
+		return false, nil
+	}
+	if err != nil {
+		return false, err
 	}
 
-	return HasElfMagic(magic)
+	return hasElfMagic(magic), nil
 }
 
 func NewElfAnalyzer(filePath string, wantedSymbols []WantedSymbol) (*ElfAnalyzer, error) {

common/elf/analyzer_test.go

@@ -2,7 +2,9 @@ package elf
 
 import (
 	"debug/elf"
+	"errors"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -252,10 +254,12 @@ func TestIsElfFile(t *testing.T) {
 		if elfPath == "" {
 			t.Skip("No ELF binary found for testing")
 		}
-		assert.True(t, IsElfFile(elfPath))
+		isElf, err := IsElfFile(elfPath)
+		require.NoError(t, err)
+		assert.True(t, isElf)
 	})
 
-	t.Run("non-ELF file", func(t *testing.T) {
+	t.Run("non-ELF file returns false nil", func(t *testing.T) {
 		tempFile, err := os.CreateTemp("", "test_non_elf_*")
 		require.NoError(t, err)
 		defer os.Remove(tempFile.Name())
@@ -264,20 +268,58 @@ func TestIsElfFile(t *testing.T) {
 		require.NoError(t, err)
 		tempFile.Close()
 
-		assert.False(t, IsElfFile(tempFile.Name()))
+		isElf, err := IsElfFile(tempFile.Name())
+		require.NoError(t, err)
+		assert.False(t, isElf)
+	})
+
+	t.Run("non-existent file returns error", func(t *testing.T) {
+		isElf, err := IsElfFile(filepath.Join(t.TempDir(), "does-not-exist"))
+		require.Error(t, err)
+		assert.True(t, errors.Is(err, os.ErrNotExist))
+		assert.False(t, isElf)
 	})
 
-	t.Run("non-existent file", func(t *testing.T) {
-		assert.False(t, IsElfFile("/non/existent/file"))
+	t.Run("permission denied returns error", func(t *testing.T) {
+		if os.Getuid() == 0 {
+			t.Skip("Running as root; permission test not meaningful")
+		}
+		tempFile, err := os.CreateTemp("", "test_noperm_*")
+		require.NoError(t, err)
+		defer os.Remove(tempFile.Name())
+		tempFile.Close()
+
+		require.NoError(t, os.Chmod(tempFile.Name(), 0o000))
+
+		isElf, err := IsElfFile(tempFile.Name())
+		require.Error(t, err)
+		assert.True(t, errors.Is(err, os.ErrPermission))
+		assert.False(t, isElf)
 	})
 
-	t.Run("empty file", func(t *testing.T) {
+	t.Run("short file returns false nil", func(t *testing.T) {
+		tempFile, err := os.CreateTemp("", "test_short_*")
+		require.NoError(t, err)
+		defer os.Remove(tempFile.Name())
+
+		_, err = tempFile.Write([]byte{0x7F, 'E'})
+		require.NoError(t, err)
+		tempFile.Close()
+
+		isElf, err := IsElfFile(tempFile.Name())
+		require.NoError(t, err)
+		assert.False(t, isElf)
+	})
+
+	t.Run("empty file returns false nil", func(t *testing.T) {
 		tempFile, err := os.CreateTemp("", "test_empty_*")
 		require.NoError(t, err)
 		defer os.Remove(tempFile.Name())
 		tempFile.Close()
 
-		assert.False(t, IsElfFile(tempFile.Name()))
+		isElf, err := IsElfFile(tempFile.Name())
+		require.NoError(t, err)
+		assert.False(t, isElf)
 	})
 }