Skip to content

Commit 4b11525

Browse files
✨ 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]>
1 parent cb565cd commit 4b11525

14 files changed

+862
-2
lines changed

docs/probes.md

+14
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,20 @@ The probe returns a single OutcomeNotApplicable if the projects has had no pull
649649
The probe returns 1 true outcome if the project has no workflows "write" permissions a the "top" level.
650650

651651

652+
## unsafeblock
653+
654+
**Lifecycle**: experimental
655+
656+
**Description**: Flags unsafe blocks of code in this project.
657+
658+
**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.
659+
660+
**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.
661+
662+
**Outcomes**: For supported ecosystem, the probe returns OutcomeTrue per unsafe block.
663+
If the project has no unsafe blocks, the probe returns OutcomeFalse.
664+
665+
652666
## webhooksUseSecrets
653667

654668
**Lifecycle**: experimental

internal/dotnet/csproj/csproj.go

+14-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ var errInvalidCsProjFile = errors.New("error parsing csproj file")
2424
type PropertyGroup struct {
2525
XMLName xml.Name `xml:"PropertyGroup"`
2626
RestoreLockedMode bool `xml:"RestoreLockedMode"`
27+
AllowUnsafeBlocks bool `xml:"AllowUnsafeBlocks"`
2728
}
2829

2930
type Project struct {
@@ -32,6 +33,18 @@ type Project struct {
3233
}
3334

3435
func IsRestoreLockedModeEnabled(content []byte) (bool, error) {
36+
return isCsProjFilePropertyGroupEnabled(content, func(propertyGroup *PropertyGroup) bool {
37+
return propertyGroup.RestoreLockedMode
38+
})
39+
}
40+
41+
func IsAllowUnsafeBlocksEnabled(content []byte) (bool, error) {
42+
return isCsProjFilePropertyGroupEnabled(content, func(propertyGroup *PropertyGroup) bool {
43+
return propertyGroup.AllowUnsafeBlocks
44+
})
45+
}
46+
47+
func isCsProjFilePropertyGroupEnabled(content []byte, predicate func(*PropertyGroup) bool) (bool, error) {
3548
var project Project
3649

3750
err := xml.Unmarshal(content, &project)
@@ -40,7 +53,7 @@ func IsRestoreLockedModeEnabled(content []byte) (bool, error) {
4053
}
4154

4255
for _, propertyGroup := range project.PropertyGroups {
43-
if propertyGroup.RestoreLockedMode {
56+
if predicate(&propertyGroup) {
4457
return true, nil
4558
}
4659
}

probes/entries.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ import (
6363
"github.com/ossf/scorecard/v5/probes/securityPolicyPresent"
6464
"github.com/ossf/scorecard/v5/probes/testsRunInCI"
6565
"github.com/ossf/scorecard/v5/probes/topLevelPermissions"
66+
"github.com/ossf/scorecard/v5/probes/unsafeblock"
6667
"github.com/ossf/scorecard/v5/probes/webhooksUseSecrets"
6768
)
6869

@@ -173,7 +174,9 @@ var (
173174
}
174175

175176
// Probes which don't use pre-computed raw data but rather collect it themselves.
176-
Independent = []IndependentProbeImpl{}
177+
Independent = []IndependentProbeImpl{
178+
unsafeblock.Run,
179+
}
177180
)
178181

179182
//nolint:gochecknoinits

probes/unsafeblock/def.yml

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Copyright 2025 OpenSSF Scorecard Authors
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
id: unsafeblock
16+
lifecycle: experimental
17+
short: Flags unsafe blocks of code in this project.
18+
motivation: >
19+
Memory safety in software should be considered a continuum, rather than being binary.
20+
While some languages and tools are memory safe by default, it may still be possible, and sometimes unavoidable, to write unsafe code in them.
21+
Unsafe code allow developers to bypass normal safety checks and directly manipulate memory.
22+
implementation: >
23+
The probe is ecosystem-specific and will surface non memory safe practices in the project by identifying unsafe code blocks.
24+
Unsafe code blocks are supported in rust, go, c#, and swift, but only go and c# are supported by this probe at this time:
25+
- for go the probe will look for the use of the `unsafe` include directive.
26+
- for c# the probe will look at the csproj and identify the use of the `AllowUnsafeBlocks` property.
27+
outcome:
28+
- For supported ecosystem, the probe returns OutcomeTrue per unsafe block.
29+
- If the project has no unsafe blocks, the probe returns OutcomeFalse.
30+
remediation:
31+
onOutcome: True
32+
effort: Medium
33+
text:
34+
- Visit the OpenSSF Memory Safety SIG guidance on how to make your project memory safe.
35+
- Guidance for [Memory-Safe By Default Languages](https://github.com/ossf/Memory-Safety/blob/main/docs/best-practice-memory-safe-by-default-languages.md)
36+
- 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)
37+
ecosystem:
38+
languages:
39+
- go
40+
- c#
41+
clients:
42+
- github
43+
- gitlab
44+
- localdir

probes/unsafeblock/impl.go

+210
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
// Copyright 2025 OpenSSF Scorecard Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package unsafeblock
16+
17+
import (
18+
"embed"
19+
"fmt"
20+
"go/parser"
21+
"go/token"
22+
"reflect"
23+
"strings"
24+
25+
"github.com/ossf/scorecard/v5/checker"
26+
"github.com/ossf/scorecard/v5/checks/fileparser"
27+
"github.com/ossf/scorecard/v5/clients"
28+
"github.com/ossf/scorecard/v5/finding"
29+
"github.com/ossf/scorecard/v5/internal/dotnet/csproj"
30+
"github.com/ossf/scorecard/v5/internal/probes"
31+
)
32+
33+
//go:embed *.yml
34+
var fs embed.FS
35+
36+
const (
37+
Probe = "unsafeblock"
38+
)
39+
40+
type languageMemoryCheckConfig struct {
41+
funcPointer func(client *checker.CheckRequest) ([]finding.Finding, error)
42+
Desc string
43+
}
44+
45+
var languageMemorySafeSpecs = map[clients.LanguageName]languageMemoryCheckConfig{
46+
clients.Go: {
47+
funcPointer: checkGoUnsafePackage,
48+
Desc: "Check if Go code uses the unsafe package",
49+
},
50+
51+
clients.CSharp: {
52+
funcPointer: checkDotnetAllowUnsafeBlocks,
53+
Desc: "Check if C# code uses unsafe blocks",
54+
},
55+
}
56+
57+
func init() {
58+
probes.MustRegisterIndependent(Probe, Run)
59+
}
60+
61+
func Run(raw *checker.CheckRequest) (found []finding.Finding, probeName string, err error) {
62+
repoLanguageChecks, err := getLanguageChecks(raw)
63+
if err != nil {
64+
return nil, Probe, err
65+
}
66+
findings := []finding.Finding{}
67+
for _, lang := range repoLanguageChecks {
68+
langFindings, err := lang.funcPointer(raw)
69+
if err != nil {
70+
return nil, Probe, fmt.Errorf("error while running function for language %s: %w", lang.Desc, err)
71+
}
72+
findings = append(findings, langFindings...)
73+
}
74+
var nonErrorFindings bool
75+
for _, f := range findings {
76+
if f.Outcome != finding.OutcomeError {
77+
nonErrorFindings = true
78+
}
79+
}
80+
// if we don't have any findings (ignoring OutcomeError), we think it's safe
81+
if !nonErrorFindings {
82+
found, err := finding.NewWith(fs, Probe,
83+
"All supported ecosystems do not declare or use unsafe code blocks", nil, finding.OutcomeFalse)
84+
if err != nil {
85+
return nil, Probe, fmt.Errorf("create finding: %w", err)
86+
}
87+
findings = append(findings, *found)
88+
}
89+
return findings, Probe, nil
90+
}
91+
92+
func getLanguageChecks(raw *checker.CheckRequest) ([]languageMemoryCheckConfig, error) {
93+
langs, err := raw.RepoClient.ListProgrammingLanguages()
94+
if err != nil {
95+
return nil, fmt.Errorf("cannot get langs of repo: %w", err)
96+
}
97+
if len(langs) == 1 && langs[0].Name == clients.All {
98+
return getAllLanguages(), nil
99+
}
100+
ret := []languageMemoryCheckConfig{}
101+
for _, language := range langs {
102+
if lang, ok := languageMemorySafeSpecs[clients.LanguageName(strings.ToLower(string(language.Name)))]; ok {
103+
ret = append(ret, lang)
104+
}
105+
}
106+
return ret, nil
107+
}
108+
109+
func getAllLanguages() []languageMemoryCheckConfig {
110+
allLanguages := make([]languageMemoryCheckConfig, 0, len(languageMemorySafeSpecs))
111+
for l := range languageMemorySafeSpecs {
112+
allLanguages = append(allLanguages, languageMemorySafeSpecs[l])
113+
}
114+
return allLanguages
115+
}
116+
117+
// Golang
118+
119+
func checkGoUnsafePackage(client *checker.CheckRequest) ([]finding.Finding, error) {
120+
findings := []finding.Finding{}
121+
if err := fileparser.OnMatchingFileContentDo(client.RepoClient, fileparser.PathMatcher{
122+
Pattern: "*.go",
123+
CaseSensitive: false,
124+
}, goCodeUsesUnsafePackage, &findings); err != nil {
125+
return nil, err
126+
}
127+
128+
return findings, nil
129+
}
130+
131+
func goCodeUsesUnsafePackage(path string, content []byte, args ...interface{}) (bool, error) {
132+
findings, ok := args[0].(*[]finding.Finding)
133+
if !ok {
134+
// panic if it is not correct type
135+
panic(fmt.Sprintf("expected type findings, got %v", reflect.TypeOf(args[0])))
136+
}
137+
fset := token.NewFileSet()
138+
f, err := parser.ParseFile(fset, "", content, parser.ImportsOnly)
139+
if err != nil {
140+
found, err := finding.NewWith(fs, Probe, "malformed golang file", &finding.Location{
141+
Path: path,
142+
}, finding.OutcomeError)
143+
if err != nil {
144+
return false, fmt.Errorf("create finding: %w", err)
145+
}
146+
*findings = append(*findings, *found)
147+
return true, nil
148+
}
149+
for _, i := range f.Imports {
150+
if i.Path.Value == `"unsafe"` {
151+
lineStart := uint(fset.Position(i.Pos()).Line)
152+
found, err := finding.NewWith(fs, Probe,
153+
"Golang code uses the unsafe package", &finding.Location{
154+
Path: path, LineStart: &lineStart,
155+
}, finding.OutcomeTrue)
156+
if err != nil {
157+
return false, fmt.Errorf("create finding: %w", err)
158+
}
159+
*findings = append(*findings, *found)
160+
}
161+
}
162+
163+
return true, nil
164+
}
165+
166+
// CSharp
167+
168+
func checkDotnetAllowUnsafeBlocks(client *checker.CheckRequest) ([]finding.Finding, error) {
169+
findings := []finding.Finding{}
170+
171+
if err := fileparser.OnMatchingFileContentDo(client.RepoClient, fileparser.PathMatcher{
172+
Pattern: "*.csproj",
173+
CaseSensitive: false,
174+
}, csProjAllosUnsafeBlocks, &findings); err != nil {
175+
return nil, err
176+
}
177+
return findings, nil
178+
}
179+
180+
func csProjAllosUnsafeBlocks(path string, content []byte, args ...interface{}) (bool, error) {
181+
findings, ok := args[0].(*[]finding.Finding)
182+
if !ok {
183+
// panic if it is not correct type
184+
panic(fmt.Sprintf("expected type findings, got %v", reflect.TypeOf(args[0])))
185+
}
186+
187+
unsafe, err := csproj.IsAllowUnsafeBlocksEnabled(content)
188+
if err != nil {
189+
found, err := finding.NewWith(fs, Probe, "malformed csproj file", &finding.Location{
190+
Path: path,
191+
}, finding.OutcomeError)
192+
if err != nil {
193+
return false, fmt.Errorf("create finding: %w", err)
194+
}
195+
*findings = append(*findings, *found)
196+
return true, nil
197+
}
198+
if unsafe {
199+
found, err := finding.NewWith(fs, Probe,
200+
"C# project file allows the use of unsafe blocks", &finding.Location{
201+
Path: path,
202+
}, finding.OutcomeTrue)
203+
if err != nil {
204+
return false, fmt.Errorf("create finding: %w", err)
205+
}
206+
*findings = append(*findings, *found)
207+
}
208+
209+
return true, nil
210+
}

0 commit comments

Comments
 (0)