Skip to content

Commit f326b79

Browse files
aknuds1bergquist
andauthored
Security: Add gosec G304 auditing annotations (grafana#29578)
* Security: Add gosec G304 auditing annotations Signed-off-by: Arve Knudsen <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * Add gosec annotations Signed-off-by: Arve Knudsen <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * Add gosec annotations Signed-off-by: Arve Knudsen <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * add G304 auditing comment Signed-off-by: bergquist <[email protected]> * Add gosec annotations Signed-off-by: Arve Knudsen <[email protected]> * space Signed-off-by: bergquist <[email protected]> * Add gosec annotations Signed-off-by: Arve Knudsen <[email protected]> Co-authored-by: bergquist <[email protected]>
1 parent 69ac69b commit f326b79

File tree

24 files changed

+89
-5
lines changed

24 files changed

+89
-5
lines changed

pkg/api/avatar/avatar.go

+3
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ func newNotFound() *Avatar {
132132
avatar := &Avatar{notFound: true}
133133

134134
// load user_profile png into buffer
135+
// It's safe to ignore gosec warning G304 since the variable part of the file path comes from a configuration
136+
// variable.
137+
// nolint:gosec
135138
path := filepath.Join(setting.StaticRootPath, "img", "user_profile.png")
136139

137140
if data, err := ioutil.ReadFile(path); err != nil {

pkg/api/dashboard.go

+3
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,9 @@ func (hs *HTTPServer) GetHomeDashboard(c *models.ReqContext) Response {
341341
filePath = filepath.Join(hs.Cfg.StaticRootPath, "dashboards/home.json")
342342
}
343343

344+
// It's safe to ignore gosec warning G304 since the variable part of the file path comes from a configuration
345+
// variable
346+
// nolint:gosec
344347
file, err := os.Open(filePath)
345348
if err != nil {
346349
return Error(500, "Failed to load home dashboard", err)

pkg/cmd/grafana-cli/commands/install_command.go

+4
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,10 @@ func extractFile(file *zip.File, filePath string) (err error) {
291291
fileMode = os.FileMode(0755)
292292
}
293293

294+
// We can ignore the gosec G304 warning on this one, since the variable part of the file path stems
295+
// from command line flag "pluginsDir", and the only possible damage would be writing to the wrong directory.
296+
// If the user shouldn't be writing to this directory, they shouldn't have the permission in the file system.
297+
// nolint:gosec
294298
dst, err := os.OpenFile(filePath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fileMode)
295299
if err != nil {
296300
if os.IsPermission(err) {

pkg/cmd/grafana-cli/services/api_client.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,11 @@ func (client *GrafanaComClient) GetPlugin(pluginId, repoUrl string) (models.Plug
4444
}
4545

4646
func (client *GrafanaComClient) DownloadFile(pluginName string, tmpFile *os.File, url string, checksum string) (err error) {
47-
// Try handling url like local file path first
47+
// Try handling URL as a local file path first
4848
if _, err := os.Stat(url); err == nil {
49+
// We can ignore this gosec G304 warning since `url` stems from command line flag "pluginUrl". If the
50+
// user shouldn't be able to read the file, it should be handled through filesystem permissions.
51+
// nolint:gosec
4952
f, err := os.Open(url)
5053
if err != nil {
5154
return errutil.Wrap("Failed to read plugin archive", err)

pkg/cmd/grafana-cli/services/io_util.go

+4
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,9 @@ func (i IoUtilImp) ReadDir(path string) ([]os.FileInfo, error) {
2121
}
2222

2323
func (i IoUtilImp) ReadFile(filename string) ([]byte, error) {
24+
// We can ignore the gosec G304 warning on this one, since the variable part of the file path stems
25+
// from command line flag "pluginsDir". If the user shouldn't be reading from this directory, they shouldn't have
26+
// the permission in the file system.
27+
// nolint:gosec
2428
return ioutil.ReadFile(filename)
2529
}

pkg/cmd/grafana-cli/utils/grafana_path.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ func GetGrafanaPluginDir(currentOS string) string {
1616
return returnOsDefault(currentOS)
1717
}
1818

19-
// getGrafanaRoot tries to get root of directory when developing grafana ie repo root. It is not perfect it just
20-
// checks what is the binary path and tries to guess based on that but if it is not running in dev env you get a bogus
19+
// getGrafanaRoot tries to get root of directory when developing grafana, ie. repo root. It is not perfect, it just
20+
// checks what is the binary path and tries to guess based on that, but if it is not running in dev env you get a bogus
2121
// path back.
2222
func getGrafanaRoot() (string, error) {
2323
ex, err := os.Executable()

pkg/components/imguploader/azureblobuploader.go

+3
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ func (az *AzureBlobUploader) Upload(ctx context.Context, imageDiskPath string) (
4545
// setup client
4646
blob := NewStorageClient(az.account_name, az.account_key)
4747

48+
// We can ignore the gosec G304 warning on this one because `imageDiskPath` comes
49+
// from alert notifiers and is only used to upload images generated by alerting.
50+
// nolint:gosec
4851
file, err := os.Open(imageDiskPath)
4952
if err != nil {
5053
return "", err

pkg/components/imguploader/gcs/gcsuploader.go

+4
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,10 @@ func (u *Uploader) uploadFile(
149149
key string,
150150
) error {
151151
u.log.Debug("Opening image file", "path", imageDiskPath)
152+
153+
// We can ignore the gosec G304 warning on this one because `imageDiskPath` comes
154+
// from alert notifiers and is only used to upload images generated by alerting.
155+
// nolint:gosec
152156
fileReader, err := os.Open(imageDiskPath)
153157
if err != nil {
154158
return err

pkg/components/imguploader/s3uploader.go

+3
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ func (u *S3Uploader) Upload(ctx context.Context, imageDiskPath string) (string,
7676
key := u.path + rand + pngExt
7777
log.Debugf("Uploading image to s3. bucket = %s, path = %s", u.bucket, key)
7878

79+
// We can ignore the gosec G304 warning on this one because `imageDiskPath` comes
80+
// from alert notifiers and is only used to upload images generated by alerting.
81+
// nolint:gosec
7982
file, err := os.Open(imageDiskPath)
8083
if err != nil {
8184
return "", err

pkg/components/imguploader/webdavuploader.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func (u *WebdavUploader) PublicURL(filename string) string {
4545
return publicURL.String()
4646
}
4747

48-
func (u *WebdavUploader) Upload(ctx context.Context, pa string) (string, error) {
48+
func (u *WebdavUploader) Upload(ctx context.Context, imgToUpload string) (string, error) {
4949
url, _ := url.Parse(u.url)
5050
filename, err := util.GetRandomString(20)
5151
if err != nil {
@@ -55,7 +55,10 @@ func (u *WebdavUploader) Upload(ctx context.Context, pa string) (string, error)
5555
filename += pngExt
5656
url.Path = path.Join(url.Path, filename)
5757

58-
imgData, err := ioutil.ReadFile(pa)
58+
// We can ignore the gosec G304 warning on this one because `imgToUpload` comes
59+
// from alert notifiers and is only used to upload images generated by alerting.
60+
// nolint:gosec
61+
imgData, err := ioutil.ReadFile(imgToUpload)
5962
if err != nil {
6063
return "", err
6164
}

pkg/middleware/recovery.go

+3
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ func stack(skip int) []byte {
5252
// Print this much at least. If we can't find the source, it won't show.
5353
fmt.Fprintf(buf, "%s:%d (0x%x)\n", file, line, pc)
5454
if file != lastFile {
55+
// We can ignore the gosec G304 warning on this one because `file`
56+
// comes from the runtime.Caller() function.
57+
// nolint:gosec
5558
data, err := ioutil.ReadFile(file)
5659
if err != nil {
5760
continue

pkg/plugins/dashboards.go

+4
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ func loadPluginDashboard(pluginId, path string) (*models.Dashboard, error) {
9595
return nil, PluginNotFoundError{pluginId}
9696
}
9797

98+
// nolint:gosec
99+
// We can ignore the gosec G304 warning on this one because `plugin.PluginDir` is based
100+
// on plugin folder structure on disk and not user input. `path` comes from the
101+
// `plugin.json` configuration file for the loaded plugin
98102
dashboardFilePath := filepath.Join(plugin.PluginDir, path)
99103
reader, err := os.Open(dashboardFilePath)
100104
if err != nil {

pkg/plugins/manifest.go

+7
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ func getPluginSignatureState(log log.Logger, plugin *PluginBase) PluginSignature
8787
log.Debug("Getting signature state of plugin", "plugin", plugin.Id, "isBackend", plugin.Backend)
8888
manifestPath := filepath.Join(plugin.PluginDir, "MANIFEST.txt")
8989

90+
// nolint:gosec
91+
// We can ignore the gosec G304 warning on this one because `manifestPath` is based
92+
// on plugin the folder structure on disk and not user input.
9093
byteValue, err := ioutil.ReadFile(manifestPath)
9194
if err != nil || len(byteValue) < 10 {
9295
log.Debug("Plugin is unsigned", "id", plugin.Id)
@@ -109,6 +112,10 @@ func getPluginSignatureState(log log.Logger, plugin *PluginBase) PluginSignature
109112
for p, hash := range manifest.Files {
110113
// Open the file
111114
fp := filepath.Join(plugin.PluginDir, p)
115+
116+
// nolint:gosec
117+
// We can ignore the gosec G304 warning on this one because `fp` is based
118+
// on the manifest file for a plugin and not user input.
112119
f, err := os.Open(fp)
113120
if err != nil {
114121
return PluginSignatureModified

pkg/plugins/plugins.go

+12
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,9 @@ func (pm *PluginManager) scan(pluginDir string, requireSigned bool) error {
269269
}
270270
}
271271

272+
// nolint:gosec
273+
// We can ignore the gosec G304 warning on this one because `jsonFPath` is based
274+
// on plugin the folder structure on disk and not user input.
272275
reader, err := os.Open(jsonFPath)
273276
if err != nil {
274277
return err
@@ -332,6 +335,9 @@ func (s *PluginScanner) walker(currentPath string, f os.FileInfo, err error) err
332335
return nil
333336
}
334337

338+
// nolint:gosec
339+
// We can ignore the gosec G304 warning on this one because `currentPath` is based
340+
// on plugin the folder structure on disk and not user input.
335341
if err := s.loadPlugin(currentPath); err != nil {
336342
s.log.Error("Failed to load plugin", "error", err, "pluginPath", filepath.Dir(currentPath))
337343
s.errors = append(s.errors, err)
@@ -471,6 +477,9 @@ func GetPluginMarkdown(pluginId string, name string) ([]byte, error) {
471477
return nil, PluginNotFoundError{pluginId}
472478
}
473479

480+
// nolint:gosec
481+
// We can ignore the gosec G304 warning on this one because `plug.PluginDir` is based
482+
// on plugin the folder structure on disk and not user input.
474483
path := filepath.Join(plug.PluginDir, fmt.Sprintf("%s.md", strings.ToUpper(name)))
475484
exists, err := fs.Exists(path)
476485
if err != nil {
@@ -488,6 +497,9 @@ func GetPluginMarkdown(pluginId string, name string) ([]byte, error) {
488497
return make([]byte, 0), nil
489498
}
490499

500+
// nolint:gosec
501+
// We can ignore the gosec G304 warning on this one because `plug.PluginDir` is based
502+
// on plugin the folder structure on disk and not user input.
491503
data, err := ioutil.ReadFile(path)
492504
if err != nil {
493505
return nil, err

pkg/services/alerting/notifiers/discord.go

+3
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ func (dn *DiscordNotifier) Notify(evalContext *alerting.EvalContext) error {
159159
}
160160

161161
func (dn *DiscordNotifier) embedImage(cmd *models.SendWebhookSync, imagePath string, existingJSONBody []byte) error {
162+
// nolint:gosec
163+
// We can ignore the gosec G304 warning on this one because `imagePath` comes
164+
// from the alert `evalContext` that generates the images.
162165
f, err := os.Open(imagePath)
163166
if err != nil {
164167
if os.IsNotExist(err) {

pkg/services/alerting/notifiers/slack.go

+6
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,8 @@ func (sn *SlackNotifier) Notify(evalContext *alerting.EvalContext) error {
331331

332332
func (sn *SlackNotifier) slackFileUpload(evalContext *alerting.EvalContext, log log.Logger, url string, recipient string, token string) error {
333333
if evalContext.ImageOnDiskPath == "" {
334+
// nolint:gosec
335+
// We can ignore the gosec G304 warning on this one because `setting.HomePath` comes from Grafana's configuration file.
334336
evalContext.ImageOnDiskPath = filepath.Join(setting.HomePath, "public/img/mixed_styles.png")
335337
}
336338
log.Info("Uploading to slack via file.upload API")
@@ -360,6 +362,10 @@ func (sn *SlackNotifier) generateSlackBody(path string, token string, recipient
360362
}()
361363

362364
// Add the generated image file
365+
// We can ignore the gosec G304 warning on this one because `imagePath` comes
366+
// from the alert `evalContext` that generates the images. `evalContext` in turn derives the root of the file
367+
// path from configuration variables.
368+
// nolint:gosec
363369
f, err := os.Open(path)
364370
if err != nil {
365371
return nil, b, err

pkg/services/ldap/ldap.go

+2
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ func (server *Server) Dial() error {
9595
if server.Config.RootCACert != "" {
9696
certPool = x509.NewCertPool()
9797
for _, caCertFile := range strings.Split(server.Config.RootCACert, " ") {
98+
// nolint:gosec
99+
// We can ignore the gosec G304 warning on this one because `caCertFile` comes from ldap config.
98100
pem, err := ioutil.ReadFile(caCertFile)
99101
if err != nil {
100102
return err

pkg/services/ldap/settings.go

+2
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ func readConfig(configFile string) (*Config, error) {
115115

116116
logger.Info("LDAP enabled, reading config file", "file", configFile)
117117

118+
// nolint:gosec
119+
// We can ignore the gosec G304 warning on this one because `filename` comes from grafana configuration file
118120
fileBytes, err := ioutil.ReadFile(configFile)
119121
if err != nil {
120122
return nil, errutil.Wrap("Failed to load LDAP config file", err)

pkg/services/provisioning/dashboards/config_reader.go

+3
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ type configReader struct {
1919

2020
func (cr *configReader) parseConfigs(file os.FileInfo) ([]*config, error) {
2121
filename, _ := filepath.Abs(filepath.Join(cr.path, file.Name()))
22+
23+
// nolint:gosec
24+
// We can ignore the gosec G304 warning on this one because `filename` comes from ps.Cfg.ProvisioningPath
2225
yamlFile, err := ioutil.ReadFile(filename)
2326
if err != nil {
2427
return nil, err

pkg/services/provisioning/dashboards/file_reader.go

+2
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,8 @@ type dashboardJSONFile struct {
343343
}
344344

345345
func (fr *FileReader) readDashboardFromFile(path string, lastModified time.Time, folderID int64) (*dashboardJSONFile, error) {
346+
// nolint:gosec
347+
// We can ignore the gosec G304 warning on this one because `path` comes from the provisioning configuration file.
346348
reader, err := os.Open(path)
347349
if err != nil {
348350
return nil, err

pkg/services/provisioning/datasources/config_reader.go

+3
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ func (cr *configReader) readConfig(path string) ([]*configs, error) {
4949

5050
func (cr *configReader) parseDatasourceConfig(path string, file os.FileInfo) (*configs, error) {
5151
filename, _ := filepath.Abs(filepath.Join(path, file.Name()))
52+
53+
// nolint:gosec
54+
// We can ignore the gosec G304 warning on this one because `filename` comes from ps.Cfg.ProvisioningPath
5255
yamlFile, err := ioutil.ReadFile(filename)
5356
if err != nil {
5457
return nil, err

pkg/services/provisioning/notifiers/config_reader.go

+3
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ func (cr *configReader) readConfig(path string) ([]*notificationsAsConfig, error
6161

6262
func (cr *configReader) parseNotificationConfig(path string, file os.FileInfo) (*notificationsAsConfig, error) {
6363
filename, _ := filepath.Abs(filepath.Join(path, file.Name()))
64+
65+
// nolint:gosec
66+
// We can ignore the gosec G304 warning on this one because `filename` comes from ps.Cfg.ProvisioningPath
6467
yamlFile, err := ioutil.ReadFile(filename)
6568
if err != nil {
6669
return nil, err

pkg/services/provisioning/plugins/config_reader.go

+2
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ func (cr *configReaderImpl) parsePluginConfig(path string, file os.FileInfo) (*p
6969
return nil, err
7070
}
7171

72+
// nolint:gosec
73+
// We can ignore the gosec G304 warning on this one because `filename` comes from ps.Cfg.ProvisioningPath
7274
yamlFile, err := ioutil.ReadFile(filename)
7375
if err != nil {
7476
return nil, err

pkg/setting/expanders.go

+2
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ func (e fileExpander) Expand(s string) (string, error) {
138138
return "", err
139139
}
140140

141+
// nolint:gosec
142+
// We can ignore the gosec G304 warning on this one because `s` comes from configuration section keys
141143
f, err := ioutil.ReadFile(s)
142144
if err != nil {
143145
return "", err

0 commit comments

Comments
 (0)