✨ New Probe: Memory safety (#4499)

* draft code Signed-off-by: balteravishay <[email protected]> * update definition.yaml Signed-off-by: balteravishay <[email protected]> * remove prominent languages code Signed-off-by: balteravishay <[email protected]> * tests Signed-off-by: balteravishay <[email protected]> * more implementation details Signed-off-by: balteravishay <[email protected]> * more coverage Signed-off-by: balteravishay <[email protected]> * more tests Signed-off-by: balteravishay <[email protected]> * fix list Signed-off-by: balteravishay <[email protected]> * fix search in map Signed-off-by: balteravishay <[email protected]> * pr comments Signed-off-by: balteravishay <[email protected]> * change to be specific for unsafe blocks Signed-off-by: balteravishay <[email protected]> * pr fixes Signed-off-by: balteravishay <[email protected]> * probes.md Signed-off-by: balteravishay <[email protected]> * return finding.OutcomeError when encountering parse errors Previously, this relied on the use of a detail logger, but we're not guaranteed to have one when running probes on their own. Instead, probes usually convey parse errors as OutcomeError. This change also required updating the test runner, as OnMatchingFileContentDo makes use of ListFiles under the hood. So simply returning a list of files was causing issues where csproj files were being parsed as Go files and vice versa. Signed-off-by: Spencer Schrock <[email protected]> --------- Signed-off-by: balteravishay <[email protected]> Signed-off-by: Spencer Schrock <[email protected]>
ossf · Feb 27, 2025 · 4b11525 · 4b11525
1 parent cb565cd
commit 4b11525
Show file tree

Hide file tree

Showing 14 changed files with 862 additions and 2 deletions.
diff --git a/docs/probes.md b/docs/probes.md
@@ -649,6 +649,20 @@ The probe returns a single OutcomeNotApplicable if the projects has had no pull
 The probe returns 1 true outcome if the project has no workflows "write" permissions a the "top" level.
 
 
+## unsafeblock
+
+**Lifecycle**: experimental
+
+**Description**: Flags unsafe blocks of code in this project.
+
+**Motivation**: Memory safety in software should be considered a continuum, rather than being binary.  While some languages and tools are memory safe by default, it may still be possible, and sometimes unavoidable, to write unsafe code in them. Unsafe code allow developers to bypass normal safety checks and directly manipulate memory.
+
+**Implementation**: The probe is ecosystem-specific and will surface non memory safe practices in the project by identifying unsafe code blocks. Unsafe code blocks are supported in rust, go, c#, and swift, but only go and c# are supported by this probe at this time: - for go the probe will look for the use of the `unsafe` include directive. - for c# the probe will look at the csproj and identify the use of the `AllowUnsafeBlocks` property.
+
+**Outcomes**: For supported ecosystem, the probe returns OutcomeTrue per unsafe block.
+If the project has no unsafe blocks, the probe returns OutcomeFalse.
+
+
 ## webhooksUseSecrets
 
 **Lifecycle**: experimental

diff --git a/internal/dotnet/csproj/csproj.go b/internal/dotnet/csproj/csproj.go
@@ -24,6 +24,7 @@ var errInvalidCsProjFile = errors.New("error parsing csproj file")
 type PropertyGroup struct {
 	XMLName           xml.Name `xml:"PropertyGroup"`
 	RestoreLockedMode bool     `xml:"RestoreLockedMode"`
+	AllowUnsafeBlocks bool     `xml:"AllowUnsafeBlocks"`
 }
 
 type Project struct {
@@ -32,6 +33,18 @@ type Project struct {
 }
 
 func IsRestoreLockedModeEnabled(content []byte) (bool, error) {
+	return isCsProjFilePropertyGroupEnabled(content, func(propertyGroup *PropertyGroup) bool {
+		return propertyGroup.RestoreLockedMode
+	})
+}
+
+func IsAllowUnsafeBlocksEnabled(content []byte) (bool, error) {
+	return isCsProjFilePropertyGroupEnabled(content, func(propertyGroup *PropertyGroup) bool {
+		return propertyGroup.AllowUnsafeBlocks
+	})
+}
+
+func isCsProjFilePropertyGroupEnabled(content []byte, predicate func(*PropertyGroup) bool) (bool, error) {
 	var project Project
 
 	err := xml.Unmarshal(content, &project)
@@ -40,7 +53,7 @@ func IsRestoreLockedModeEnabled(content []byte) (bool, error) {
 	}
 
 	for _, propertyGroup := range project.PropertyGroups {
-		if propertyGroup.RestoreLockedMode {
+		if predicate(&propertyGroup) {
 			return true, nil
 		}
 	}

diff --git a/probes/entries.go b/probes/entries.go
@@ -63,6 +63,7 @@ import (
 	"github.com/ossf/scorecard/v5/probes/securityPolicyPresent"
 	"github.com/ossf/scorecard/v5/probes/testsRunInCI"
 	"github.com/ossf/scorecard/v5/probes/topLevelPermissions"
+	"github.com/ossf/scorecard/v5/probes/unsafeblock"
 	"github.com/ossf/scorecard/v5/probes/webhooksUseSecrets"
 )
 
@@ -173,7 +174,9 @@ var (
 	}
 
 	// Probes which don't use pre-computed raw data but rather collect it themselves.
-	Independent = []IndependentProbeImpl{}
+	Independent = []IndependentProbeImpl{
+		unsafeblock.Run,
+	}
 )
 
 //nolint:gochecknoinits

diff --git a/probes/unsafeblock/def.yml b/probes/unsafeblock/def.yml
@@ -0,0 +1,44 @@
+# Copyright 2025 OpenSSF Scorecard Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+id: unsafeblock
+lifecycle: experimental
+short: Flags unsafe blocks of code in this project.
+motivation: >
+  Memory safety in software should be considered a continuum, rather than being binary. 
+  While some languages and tools are memory safe by default, it may still be possible, and sometimes unavoidable, to write unsafe code in them.
+  Unsafe code allow developers to bypass normal safety checks and directly manipulate memory.
+implementation: >
+  The probe is ecosystem-specific and will surface non memory safe practices in the project by identifying unsafe code blocks.
+  Unsafe code blocks are supported in rust, go, c#, and swift, but only go and c# are supported by this probe at this time:
+  - for go the probe will look for the use of the `unsafe` include directive.
+  - for c# the probe will look at the csproj and identify the use of the `AllowUnsafeBlocks` property.
+outcome:
+  - For supported ecosystem, the probe returns OutcomeTrue per unsafe block.
+  - If the project has no unsafe blocks, the probe returns OutcomeFalse.
+remediation:
+  onOutcome: True
+  effort: Medium
+  text:
+    - Visit the OpenSSF Memory Safety SIG guidance on how to make your project memory safe.
+    - Guidance for [Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-memory-safe-by-default-languages.md)
+    - Guidance for [Non Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-non-memory-safe-by-default-languages.md)
+ecosystem:
+  languages:
+    - go
+    - c#
+  clients:
+    - github  
+    - gitlab  
+    - localdir  
diff --git a/probes/unsafeblock/impl.go b/probes/unsafeblock/impl.go
@@ -0,0 +1,210 @@
+// Copyright 2025 OpenSSF Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package unsafeblock
+
+import (
+	"embed"
+	"fmt"
+	"go/parser"
+	"go/token"
+	"reflect"
+	"strings"
+
+	"github.com/ossf/scorecard/v5/checker"
+	"github.com/ossf/scorecard/v5/checks/fileparser"
+	"github.com/ossf/scorecard/v5/clients"
+	"github.com/ossf/scorecard/v5/finding"
+	"github.com/ossf/scorecard/v5/internal/dotnet/csproj"
+	"github.com/ossf/scorecard/v5/internal/probes"
+)
+
+//go:embed *.yml
+var fs embed.FS
+
+const (
+	Probe = "unsafeblock"
+)
+
+type languageMemoryCheckConfig struct {
+	funcPointer func(client *checker.CheckRequest) ([]finding.Finding, error)
+	Desc        string
+}
+
+var languageMemorySafeSpecs = map[clients.LanguageName]languageMemoryCheckConfig{
+	clients.Go: {
+		funcPointer: checkGoUnsafePackage,
+		Desc:        "Check if Go code uses the unsafe package",
+	},
+
+	clients.CSharp: {
+		funcPointer: checkDotnetAllowUnsafeBlocks,
+		Desc:        "Check if C# code uses unsafe blocks",
+	},
+}
+
+func init() {
+	probes.MustRegisterIndependent(Probe, Run)
+}
+
+func Run(raw *checker.CheckRequest) (found []finding.Finding, probeName string, err error) {
+	repoLanguageChecks, err := getLanguageChecks(raw)
+	if err != nil {
+		return nil, Probe, err
+	}
+	findings := []finding.Finding{}
+	for _, lang := range repoLanguageChecks {
+		langFindings, err := lang.funcPointer(raw)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("error while running function for language %s: %w", lang.Desc, err)
+		}
+		findings = append(findings, langFindings...)
+	}
+	var nonErrorFindings bool
+	for _, f := range findings {
+		if f.Outcome != finding.OutcomeError {
+			nonErrorFindings = true
+		}
+	}
+	// if we don't have any findings (ignoring OutcomeError), we think it's safe
+	if !nonErrorFindings {
+		found, err := finding.NewWith(fs, Probe,
+			"All supported ecosystems do not declare or use unsafe code blocks", nil, finding.OutcomeFalse)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
+		}
+		findings = append(findings, *found)
+	}
+	return findings, Probe, nil
+}
+
+func getLanguageChecks(raw *checker.CheckRequest) ([]languageMemoryCheckConfig, error) {
+	langs, err := raw.RepoClient.ListProgrammingLanguages()
+	if err != nil {
+		return nil, fmt.Errorf("cannot get langs of repo: %w", err)
+	}
+	if len(langs) == 1 && langs[0].Name == clients.All {
+		return getAllLanguages(), nil
+	}
+	ret := []languageMemoryCheckConfig{}
+	for _, language := range langs {
+		if lang, ok := languageMemorySafeSpecs[clients.LanguageName(strings.ToLower(string(language.Name)))]; ok {
+			ret = append(ret, lang)
+		}
+	}
+	return ret, nil
+}
+
+func getAllLanguages() []languageMemoryCheckConfig {
+	allLanguages := make([]languageMemoryCheckConfig, 0, len(languageMemorySafeSpecs))
+	for l := range languageMemorySafeSpecs {
+		allLanguages = append(allLanguages, languageMemorySafeSpecs[l])
+	}
+	return allLanguages
+}
+
+// Golang
+
+func checkGoUnsafePackage(client *checker.CheckRequest) ([]finding.Finding, error) {
+	findings := []finding.Finding{}
+	if err := fileparser.OnMatchingFileContentDo(client.RepoClient, fileparser.PathMatcher{
+		Pattern:       "*.go",
+		CaseSensitive: false,
+	}, goCodeUsesUnsafePackage, &findings); err != nil {
+		return nil, err
+	}
+
+	return findings, nil
+}
+
+func goCodeUsesUnsafePackage(path string, content []byte, args ...interface{}) (bool, error) {
+	findings, ok := args[0].(*[]finding.Finding)
+	if !ok {
+		// panic if it is not correct type
+		panic(fmt.Sprintf("expected type findings, got %v", reflect.TypeOf(args[0])))
+	}
+	fset := token.NewFileSet()
+	f, err := parser.ParseFile(fset, "", content, parser.ImportsOnly)
+	if err != nil {
+		found, err := finding.NewWith(fs, Probe, "malformed golang file", &finding.Location{
+			Path: path,
+		}, finding.OutcomeError)
+		if err != nil {
+			return false, fmt.Errorf("create finding: %w", err)
+		}
+		*findings = append(*findings, *found)
+		return true, nil
+	}
+	for _, i := range f.Imports {
+		if i.Path.Value == `"unsafe"` {
+			lineStart := uint(fset.Position(i.Pos()).Line)
+			found, err := finding.NewWith(fs, Probe,
+				"Golang code uses the unsafe package", &finding.Location{
+					Path: path, LineStart: &lineStart,
+				}, finding.OutcomeTrue)
+			if err != nil {
+				return false, fmt.Errorf("create finding: %w", err)
+			}
+			*findings = append(*findings, *found)
+		}
+	}
+
+	return true, nil
+}
+
+// CSharp
+
+func checkDotnetAllowUnsafeBlocks(client *checker.CheckRequest) ([]finding.Finding, error) {
+	findings := []finding.Finding{}
+
+	if err := fileparser.OnMatchingFileContentDo(client.RepoClient, fileparser.PathMatcher{
+		Pattern:       "*.csproj",
+		CaseSensitive: false,
+	}, csProjAllosUnsafeBlocks, &findings); err != nil {
+		return nil, err
+	}
+	return findings, nil
+}
+
+func csProjAllosUnsafeBlocks(path string, content []byte, args ...interface{}) (bool, error) {
+	findings, ok := args[0].(*[]finding.Finding)
+	if !ok {
+		// panic if it is not correct type
+		panic(fmt.Sprintf("expected type findings, got %v", reflect.TypeOf(args[0])))
+	}
+
+	unsafe, err := csproj.IsAllowUnsafeBlocksEnabled(content)
+	if err != nil {
+		found, err := finding.NewWith(fs, Probe, "malformed csproj file", &finding.Location{
+			Path: path,
+		}, finding.OutcomeError)
+		if err != nil {
+			return false, fmt.Errorf("create finding: %w", err)
+		}
+		*findings = append(*findings, *found)
+		return true, nil
+	}
+	if unsafe {
+		found, err := finding.NewWith(fs, Probe,
+			"C# project file allows the use of unsafe blocks", &finding.Location{
+				Path: path,
+			}, finding.OutcomeTrue)
+		if err != nil {
+			return false, fmt.Errorf("create finding: %w", err)
+		}
+		*findings = append(*findings, *found)
+	}
+
+	return true, nil
+}