Skip to content

Commit

Permalink
Security: Add gosec G304 auditing annotations (#29578)
Browse files Browse the repository at this point in the history
* Security: Add gosec G304 auditing annotations

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* Add gosec annotations

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* Add gosec annotations

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* add G304 auditing comment

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* Add gosec annotations

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* space

Signed-off-by: bergquist <carl.bergquist@gmail.com>

* Add gosec annotations

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Co-authored-by: bergquist <carl.bergquist@gmail.com>
  • Loading branch information
aknuds1 and bergquist committed Dec 3, 2020
1 parent 69ac69b commit f326b79
Show file tree
Hide file tree
Showing 24 changed files with 89 additions and 5 deletions.
3 changes: 3 additions & 0 deletions pkg/api/avatar/avatar.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ func newNotFound() *Avatar {
avatar := &Avatar{notFound: true}

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

if data, err := ioutil.ReadFile(path); err != nil {
Expand Down
3 changes: 3 additions & 0 deletions pkg/api/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,9 @@ func (hs *HTTPServer) GetHomeDashboard(c *models.ReqContext) Response {
filePath = filepath.Join(hs.Cfg.StaticRootPath, "dashboards/home.json")
}

// It's safe to ignore gosec warning G304 since the variable part of the file path comes from a configuration
// variable
// nolint:gosec
file, err := os.Open(filePath)
if err != nil {
return Error(500, "Failed to load home dashboard", err)
Expand Down
4 changes: 4 additions & 0 deletions pkg/cmd/grafana-cli/commands/install_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@ func extractFile(file *zip.File, filePath string) (err error) {
fileMode = os.FileMode(0755)
}

// We can ignore the gosec G304 warning on this one, since the variable part of the file path stems
// from command line flag "pluginsDir", and the only possible damage would be writing to the wrong directory.
// If the user shouldn't be writing to this directory, they shouldn't have the permission in the file system.
// nolint:gosec
dst, err := os.OpenFile(filePath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fileMode)
if err != nil {
if os.IsPermission(err) {
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/grafana-cli/services/api_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,11 @@ func (client *GrafanaComClient) GetPlugin(pluginId, repoUrl string) (models.Plug
}

func (client *GrafanaComClient) DownloadFile(pluginName string, tmpFile *os.File, url string, checksum string) (err error) {
// Try handling url like local file path first
// Try handling URL as a local file path first
if _, err := os.Stat(url); err == nil {
// We can ignore this gosec G304 warning since `url` stems from command line flag "pluginUrl". If the
// user shouldn't be able to read the file, it should be handled through filesystem permissions.
// nolint:gosec
f, err := os.Open(url)
if err != nil {
return errutil.Wrap("Failed to read plugin archive", err)
Expand Down
4 changes: 4 additions & 0 deletions pkg/cmd/grafana-cli/services/io_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,9 @@ func (i IoUtilImp) ReadDir(path string) ([]os.FileInfo, error) {
}

func (i IoUtilImp) ReadFile(filename string) ([]byte, error) {
// We can ignore the gosec G304 warning on this one, since the variable part of the file path stems
// from command line flag "pluginsDir". If the user shouldn't be reading from this directory, they shouldn't have
// the permission in the file system.
// nolint:gosec
return ioutil.ReadFile(filename)
}
4 changes: 2 additions & 2 deletions pkg/cmd/grafana-cli/utils/grafana_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ func GetGrafanaPluginDir(currentOS string) string {
return returnOsDefault(currentOS)
}

// getGrafanaRoot tries to get root of directory when developing grafana ie repo root. It is not perfect it just
// 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
// getGrafanaRoot tries to get root of directory when developing grafana, ie. repo root. It is not perfect, it just
// 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
// path back.
func getGrafanaRoot() (string, error) {
ex, err := os.Executable()
Expand Down
3 changes: 3 additions & 0 deletions pkg/components/imguploader/azureblobuploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ func (az *AzureBlobUploader) Upload(ctx context.Context, imageDiskPath string) (
// setup client
blob := NewStorageClient(az.account_name, az.account_key)

// We can ignore the gosec G304 warning on this one because `imageDiskPath` comes
// from alert notifiers and is only used to upload images generated by alerting.
// nolint:gosec
file, err := os.Open(imageDiskPath)
if err != nil {
return "", err
Expand Down
4 changes: 4 additions & 0 deletions pkg/components/imguploader/gcs/gcsuploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ func (u *Uploader) uploadFile(
key string,
) error {
u.log.Debug("Opening image file", "path", imageDiskPath)

// We can ignore the gosec G304 warning on this one because `imageDiskPath` comes
// from alert notifiers and is only used to upload images generated by alerting.
// nolint:gosec
fileReader, err := os.Open(imageDiskPath)
if err != nil {
return err
Expand Down
3 changes: 3 additions & 0 deletions pkg/components/imguploader/s3uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ func (u *S3Uploader) Upload(ctx context.Context, imageDiskPath string) (string,
key := u.path + rand + pngExt
log.Debugf("Uploading image to s3. bucket = %s, path = %s", u.bucket, key)

// We can ignore the gosec G304 warning on this one because `imageDiskPath` comes
// from alert notifiers and is only used to upload images generated by alerting.
// nolint:gosec
file, err := os.Open(imageDiskPath)
if err != nil {
return "", err
Expand Down
7 changes: 5 additions & 2 deletions pkg/components/imguploader/webdavuploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (u *WebdavUploader) PublicURL(filename string) string {
return publicURL.String()
}

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

imgData, err := ioutil.ReadFile(pa)
// We can ignore the gosec G304 warning on this one because `imgToUpload` comes
// from alert notifiers and is only used to upload images generated by alerting.
// nolint:gosec
imgData, err := ioutil.ReadFile(imgToUpload)
if err != nil {
return "", err
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/middleware/recovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ func stack(skip int) []byte {
// Print this much at least. If we can't find the source, it won't show.
fmt.Fprintf(buf, "%s:%d (0x%x)\n", file, line, pc)
if file != lastFile {
// We can ignore the gosec G304 warning on this one because `file`
// comes from the runtime.Caller() function.
// nolint:gosec
data, err := ioutil.ReadFile(file)
if err != nil {
continue
Expand Down
4 changes: 4 additions & 0 deletions pkg/plugins/dashboards.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ func loadPluginDashboard(pluginId, path string) (*models.Dashboard, error) {
return nil, PluginNotFoundError{pluginId}
}

// nolint:gosec
// We can ignore the gosec G304 warning on this one because `plugin.PluginDir` is based
// on plugin folder structure on disk and not user input. `path` comes from the
// `plugin.json` configuration file for the loaded plugin
dashboardFilePath := filepath.Join(plugin.PluginDir, path)
reader, err := os.Open(dashboardFilePath)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions pkg/plugins/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ func getPluginSignatureState(log log.Logger, plugin *PluginBase) PluginSignature
log.Debug("Getting signature state of plugin", "plugin", plugin.Id, "isBackend", plugin.Backend)
manifestPath := filepath.Join(plugin.PluginDir, "MANIFEST.txt")

// nolint:gosec
// We can ignore the gosec G304 warning on this one because `manifestPath` is based
// on plugin the folder structure on disk and not user input.
byteValue, err := ioutil.ReadFile(manifestPath)
if err != nil || len(byteValue) < 10 {
log.Debug("Plugin is unsigned", "id", plugin.Id)
Expand All @@ -109,6 +112,10 @@ func getPluginSignatureState(log log.Logger, plugin *PluginBase) PluginSignature
for p, hash := range manifest.Files {
// Open the file
fp := filepath.Join(plugin.PluginDir, p)

// nolint:gosec
// We can ignore the gosec G304 warning on this one because `fp` is based
// on the manifest file for a plugin and not user input.
f, err := os.Open(fp)
if err != nil {
return PluginSignatureModified
Expand Down
12 changes: 12 additions & 0 deletions pkg/plugins/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,9 @@ func (pm *PluginManager) scan(pluginDir string, requireSigned bool) error {
}
}

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

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

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

// nolint:gosec
// We can ignore the gosec G304 warning on this one because `plug.PluginDir` is based
// on plugin the folder structure on disk and not user input.
data, err := ioutil.ReadFile(path)
if err != nil {
return nil, err
Expand Down
3 changes: 3 additions & 0 deletions pkg/services/alerting/notifiers/discord.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ func (dn *DiscordNotifier) Notify(evalContext *alerting.EvalContext) error {
}

func (dn *DiscordNotifier) embedImage(cmd *models.SendWebhookSync, imagePath string, existingJSONBody []byte) error {
// nolint:gosec
// We can ignore the gosec G304 warning on this one because `imagePath` comes
// from the alert `evalContext` that generates the images.
f, err := os.Open(imagePath)
if err != nil {
if os.IsNotExist(err) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/services/alerting/notifiers/slack.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,8 @@ func (sn *SlackNotifier) Notify(evalContext *alerting.EvalContext) error {

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

// Add the generated image file
// We can ignore the gosec G304 warning on this one because `imagePath` comes
// from the alert `evalContext` that generates the images. `evalContext` in turn derives the root of the file
// path from configuration variables.
// nolint:gosec
f, err := os.Open(path)
if err != nil {
return nil, b, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/services/ldap/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ func (server *Server) Dial() error {
if server.Config.RootCACert != "" {
certPool = x509.NewCertPool()
for _, caCertFile := range strings.Split(server.Config.RootCACert, " ") {
// nolint:gosec
// We can ignore the gosec G304 warning on this one because `caCertFile` comes from ldap config.
pem, err := ioutil.ReadFile(caCertFile)
if err != nil {
return err
Expand Down
2 changes: 2 additions & 0 deletions pkg/services/ldap/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ func readConfig(configFile string) (*Config, error) {

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

// nolint:gosec
// We can ignore the gosec G304 warning on this one because `filename` comes from grafana configuration file
fileBytes, err := ioutil.ReadFile(configFile)
if err != nil {
return nil, errutil.Wrap("Failed to load LDAP config file", err)
Expand Down
3 changes: 3 additions & 0 deletions pkg/services/provisioning/dashboards/config_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ type configReader struct {

func (cr *configReader) parseConfigs(file os.FileInfo) ([]*config, error) {
filename, _ := filepath.Abs(filepath.Join(cr.path, file.Name()))

// nolint:gosec
// We can ignore the gosec G304 warning on this one because `filename` comes from ps.Cfg.ProvisioningPath
yamlFile, err := ioutil.ReadFile(filename)
if err != nil {
return nil, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/services/provisioning/dashboards/file_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,8 @@ type dashboardJSONFile struct {
}

func (fr *FileReader) readDashboardFromFile(path string, lastModified time.Time, folderID int64) (*dashboardJSONFile, error) {
// nolint:gosec
// We can ignore the gosec G304 warning on this one because `path` comes from the provisioning configuration file.
reader, err := os.Open(path)
if err != nil {
return nil, err
Expand Down
3 changes: 3 additions & 0 deletions pkg/services/provisioning/datasources/config_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ func (cr *configReader) readConfig(path string) ([]*configs, error) {

func (cr *configReader) parseDatasourceConfig(path string, file os.FileInfo) (*configs, error) {
filename, _ := filepath.Abs(filepath.Join(path, file.Name()))

// nolint:gosec
// We can ignore the gosec G304 warning on this one because `filename` comes from ps.Cfg.ProvisioningPath
yamlFile, err := ioutil.ReadFile(filename)
if err != nil {
return nil, err
Expand Down
3 changes: 3 additions & 0 deletions pkg/services/provisioning/notifiers/config_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ func (cr *configReader) readConfig(path string) ([]*notificationsAsConfig, error

func (cr *configReader) parseNotificationConfig(path string, file os.FileInfo) (*notificationsAsConfig, error) {
filename, _ := filepath.Abs(filepath.Join(path, file.Name()))

// nolint:gosec
// We can ignore the gosec G304 warning on this one because `filename` comes from ps.Cfg.ProvisioningPath
yamlFile, err := ioutil.ReadFile(filename)
if err != nil {
return nil, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/services/provisioning/plugins/config_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ func (cr *configReaderImpl) parsePluginConfig(path string, file os.FileInfo) (*p
return nil, err
}

// nolint:gosec
// We can ignore the gosec G304 warning on this one because `filename` comes from ps.Cfg.ProvisioningPath
yamlFile, err := ioutil.ReadFile(filename)
if err != nil {
return nil, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/setting/expanders.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ func (e fileExpander) Expand(s string) (string, error) {
return "", err
}

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

0 comments on commit f326b79

Please sign in to comment.