Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: finish addressing ~easy gosec warnings #1603

Merged
merged 8 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/ooniprobe/internal/autorun/autorun_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,11 @@ func (managerDarwin) writePlist() error {
return err
}
log.Infof("exec: mkdir -p %s", plistDir)
if err := os.MkdirAll(plistDir, 0755); err != nil {
if err := os.MkdirAll(plistDir, 0700); err != nil {
return err
}
log.Infof("exec: writePlist(%s)", plistPath)
return os.WriteFile(plistPath, out.Bytes(), 0644)
return os.WriteFile(plistPath, out.Bytes(), 0600)
}

func (managerDarwin) start() error {
Expand Down
4 changes: 3 additions & 1 deletion cmd/ooniprobe/internal/cli/reset/reset.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ func init() {
return err
}
if *force {
os.RemoveAll(ctx.Home())
// trade off: we're not checking for an error here to make the
// OONI directory deletion idempotent
_ = os.RemoveAll(ctx.Home())
log.Infof("Deleted %s", ctx.Home())
} else {
log.Infof("Run with --force to delete %s", ctx.Home())
Expand Down
5 changes: 3 additions & 2 deletions cmd/ooniprobe/internal/cli/rm/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/apex/log"
"github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/cli/root"
"github.com/ooni/probe-cli/v3/internal/database"
"github.com/ooni/probe-cli/v3/internal/runtimex"
"github.com/upper/db/v4"
)

Expand All @@ -20,7 +21,7 @@ func deleteAll(d *database.Database, skipInteractive bool) error {
Options: []string{"true", "false"},
Default: "false",
}
survey.AskOne(confirm, &answer, nil)
runtimex.Try0(survey.AskOne(confirm, &answer, nil))
if answer == "false" {
return errors.New("canceled by user")
}
Expand Down Expand Up @@ -80,7 +81,7 @@ func init() {
Options: []string{"true", "false"},
Default: "false",
}
survey.AskOne(confirm, &answer, nil)
runtimex.Try0(survey.AskOne(confirm, &answer, nil))
if answer == "false" {
return errors.New("canceled by user")
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/ooniprobe/internal/config/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (c *Config) Write() error {
if c.path == "" {
return errors.New("config file path is empty")
}
if err := os.WriteFile(c.path, configJSON, 0644); err != nil {
if err := os.WriteFile(c.path, configJSON, 0600); err != nil {
return errors.Wrap(err, "writing config JSON")
}
return nil
Expand Down
4 changes: 2 additions & 2 deletions cmd/ooniprobe/internal/nettests/nettests.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,9 @@ func (c *Controller) Run(builder model.ExperimentBuilder, inputs []string) error
return errors.Wrap(err, "failed to add test keys to summary")
}
}
db.UpdateUploadedStatus(c.res)
err := db.UpdateUploadedStatus(c.res)
log.Debugf("status.end")
return nil
return err
}

// OnProgress should be called when a new progress event is available.
Expand Down
2 changes: 1 addition & 1 deletion cmd/ooniprobe/internal/ooni/ooni.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func InitDefaultConfig(home string) (*config.Config, error) {
if err != nil {
if os.IsNotExist(err) {
log.Debugf("writing default config to %s", configPath)
if err = os.WriteFile(configPath, defaultConfig, 0644); err != nil {
if err = os.WriteFile(configPath, defaultConfig, 0600); err != nil {
return nil, err
}
// If the user did the informed consent procedure in
Expand Down
14 changes: 11 additions & 3 deletions internal/cmd/oohelperd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,16 @@ func main() {
} else {
w.Header().Set("WWW-Authenticate", "Basic realm=metrics")
w.WriteHeader(401)
w.Write([]byte("401 Unauthorized\n"))
_, _ = w.Write([]byte("401 Unauthorized\n"))
}
})

// create a listening server for serving ooniprobe requests
srv := &http.Server{Addr: *apiEndpoint, Handler: mux}
srv := &http.Server{
Addr: *apiEndpoint,
Handler: mux,
ReadHeaderTimeout: 8 * time.Second,
}
listener, err := net.Listen("tcp", *apiEndpoint)
runtimex.PanicOnError(err, "net.Listen failed")

Expand All @@ -121,7 +125,11 @@ func main() {
pprofMux := http.NewServeMux()
pprofMux.Handle("/debug/pprof/profile", http.HandlerFunc(pprof.Profile))
pprofMux.Handle("/debug/pprof/trace", http.HandlerFunc(pprof.Trace))
pprofSrv := &http.Server{Addr: *pprofEndpoint, Handler: pprofMux}
pprofSrv := &http.Server{
Addr: *pprofEndpoint,
Handler: pprofMux,
ReadHeaderTimeout: 8 * time.Second,
}
go pprofSrv.ListenAndServe()
log.Infof("serving CPU profile at http://%s/debug/pprof/profile", *pprofEndpoint)
log.Infof("serving execution traces at http://%s/debug/pprof/trace", *pprofEndpoint)
Expand Down
10 changes: 6 additions & 4 deletions internal/database/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,10 @@ func (d *Database) DeleteResult(resultID int64) error {
return err
}
if err := res.Delete(); err != nil {
log.WithError(err).Error("failed to delete the result directory")
log.WithError(err).Error("failed to delete the result")
return err
}
os.RemoveAll(result.MeasurementDir)
return nil
return os.RemoveAll(result.MeasurementDir)
}

// UpdateUploadedStatus implements WritableDatabase.UpdateUploadedStatus
Expand Down Expand Up @@ -337,7 +336,10 @@ func (d *Database) CreateOrUpdateURL(urlStr string, categoryCode string, country
return err
} else {
url.CategoryCode = sql.NullString{String: categoryCode, Valid: true}
res.Update(url)
if err := res.Update(url); err != nil {
log.WithError(err).Error("Failed to update the database")
return err
}
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/netemx/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func (srv *httpCleartextServer) mustListenPortLocked(handler http.Handler, ipAdd
listener := runtimex.Try1(srv.unet.ListenTCP("tcp", addr))

// serve requests in a background goroutine
srvr := &http.Server{Handler: handler}
srvr := &http.Server{Handler: handler} // #nosec G112 - just a testing server
go srvr.Serve(listener)

// make sure we track the server (the .Serve method will close the
Expand Down
2 changes: 1 addition & 1 deletion internal/netemx/https.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (srv *httpSecureServer) mustListenPortLocked(handler http.Handler, ipAddr n
tlsConfig := srv.unet.MustNewServerTLSConfig(srv.serverNameMain, srv.serverNameExtras...)

// serve requests in a background goroutine
srvr := &http.Server{
srvr := &http.Server{ // #nosec G112 - just a testing server
Handler: handler,
TLSConfig: tlsConfig,
}
Expand Down
4 changes: 2 additions & 2 deletions internal/testingx/httptestx.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func MustNewHTTPServerEx(addr *net.TCPAddr, httpListener TCPListener, handler ht
Path: "/",
}
srv := &HTTPServer{
Config: &http.Server{Handler: handler},
Config: &http.Server{Handler: handler}, // #nosec G112 - just a testing server
Listener: listener,
TLS: nil,
URL: baseURL.String(),
Expand Down Expand Up @@ -113,7 +113,7 @@ func MustNewHTTPServerTLSEx(
otherNames = append(otherNames, extraSNIs...)

srv := &HTTPServer{
Config: &http.Server{Handler: handler},
Config: &http.Server{Handler: handler}, // #nosec G112 - just a testing server
Listener: listener,
TLS: ca.MustNewServerTLSConfig(commonName, otherNames...),
URL: baseURL.String(),
Expand Down
2 changes: 1 addition & 1 deletion pkg/gobash/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func unpackZip(targetDir, archiveFile string) error {
}

// File
if err := os.MkdirAll(filepath.Dir(outpath), 0755); err != nil {
if err := os.MkdirAll(filepath.Dir(outpath), 0700); err != nil {
return err
}
out, err := os.OpenFile( // #nosec G304 - this is working as intended
Expand Down
Loading