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

security: fix issues reported by new version of gosec #254

Merged
merged 2 commits into from
Sep 28, 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
2 changes: 1 addition & 1 deletion cmd/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func runCreate(cmd *cobra.Command, _ []string) {
var file *os.File

if writePath != "" {
file, err = os.OpenFile(writePath, os.O_CREATE|os.O_TRUNC|os.O_RDWR, 0666)
file, err = os.OpenFile(filepath.Clean(writePath), os.O_CREATE|os.O_TRUNC|os.O_RDWR, 0600)

if err != nil {
panic(err)
Expand Down
6 changes: 5 additions & 1 deletion cmd/seed.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func initSeed() {
rootCmd.AddCommand(seedCmd)

seedCmd.Flags().StringP("layer", "l", "", "The ID of the layer to seed")
seedCmd.MarkFlagRequired("layer") //nolint:errcheck
err := seedCmd.MarkFlagRequired("layer")
seedCmd.Flags().BoolP("verbose", "v", false, "Output verbose information including every tile being requested and success or error status")
seedCmd.Flags().UintSliceP("zoom", "z", []uint{0, 1, 2, 3, 4, 5}, "The zoom level(s) to seed")
seedCmd.Flags().Float32P("min-latitude", "s", -90, "The minimum latitude to seed. The south side of the bounding box")
Expand All @@ -97,4 +97,8 @@ func initSeed() {
seedCmd.Flags().Bool("force", false, "Perform the seeding even if it'll produce an excessive number of tiles. Without this flag seeds over 10k tiles will error out. \nWarning: Overriding this protection absolutely can cause an Out-of-Memory error")
seedCmd.Flags().Uint16P("threads", "t", 1, "How many concurrent requests to use to perform seeding. Be mindful of spamming upstream providers")
// TODO: support some way to support writing just to a specific cache when Multi cache is being used

if err != nil {
panic(err)
}
}
14 changes: 7 additions & 7 deletions cmd/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ Example:

func runTest(cmd *cobra.Command, _ []string) {
layerNames, err1 := cmd.Flags().GetStringSlice("layer")
z, err2 := cmd.Flags().GetUint("z-coordinate")
x, err3 := cmd.Flags().GetUint("y-coordinate")
y, err4 := cmd.Flags().GetUint("x-coordinate")
z, err2 := cmd.Flags().GetInt("z-coordinate")
x, err3 := cmd.Flags().GetInt("y-coordinate")
y, err4 := cmd.Flags().GetInt("x-coordinate")
noCache, err5 := cmd.Flags().GetBool("no-cache")
numThread, err6 := cmd.Flags().GetUint16("threads")
out := rootCmd.OutOrStdout()
Expand All @@ -59,7 +59,7 @@ func runTest(cmd *cobra.Command, _ []string) {
return
}

errCount, err := tg.Test(cfg, tg.TestOptions{LayerNames: layerNames, Z: int(z), X: int(x), Y: int(y), NumThread: numThread, NoCache: noCache}, out)
errCount, err := tg.Test(cfg, tg.TestOptions{LayerNames: layerNames, Z: z, X: x, Y: y, NumThread: numThread, NoCache: noCache}, out)

if err != nil {
fmt.Fprintf(out, "Error: %v", err)
Expand Down Expand Up @@ -87,9 +87,9 @@ func initTest() {
rootCmd.AddCommand(testCmd)

testCmd.Flags().StringSliceP("layer", "l", []string{}, "The ID(s) of the layer to test. Tests all layers by default")
testCmd.Flags().UintP("z-coordinate", "z", 10, "The z coordinate to use to test") //nolint:mnd
testCmd.Flags().UintP("x-coordinate", "x", 123, "The x coordinate to use to test") //nolint:mnd
testCmd.Flags().UintP("y-coordinate", "y", 534, "The y coordinate to use to test") //nolint:mnd
testCmd.Flags().IntP("z-coordinate", "z", 10, "The z coordinate to use to test") //nolint:mnd
testCmd.Flags().IntP("x-coordinate", "x", 123, "The x coordinate to use to test") //nolint:mnd
testCmd.Flags().IntP("y-coordinate", "y", 534, "The y coordinate to use to test") //nolint:mnd
testCmd.Flags().Bool("no-cache", false, "Don't write to the cache. The Cache configuration must still be syntactically valid")
testCmd.Flags().Uint16P("threads", "t", 1, "How many layers to test at once. Be mindful of spamming upstream providers")
//TODO: output in custom format or write to file
Expand Down
2 changes: 1 addition & 1 deletion internal/caches/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (s DiskRegistration) Initialize(configAny any, errorMessages config.ErrorMe
func (c Disk) Lookup(_ context.Context, t pkg.TileRequest) (*pkg.Image, error) {
filename := requestToFilename(t)

b, err := os.ReadFile(filepath.Join(c.Path, filename))
b, err := os.ReadFile(filepath.Clean(filepath.Join(c.Path, filename)))

if errors.Is(err, os.ErrNotExist) {
return nil, nil
Expand Down
4 changes: 2 additions & 2 deletions internal/caches/memcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type MemcacheConfig struct {
HostAndPort `mapstructure:",squash"`
Servers []HostAndPort // The list of servers to use.
KeyPrefix string // Prefix to keynames stored in cache
TTL uint32 // Cache expiration in seconds. Max of 30 days. Default to 1 day
TTL uint // Cache expiration in seconds. Max of 30 days. Default to 1 day
}

type Memcache struct {
Expand Down Expand Up @@ -107,5 +107,5 @@ func (c Memcache) Save(_ context.Context, t pkg.TileRequest, img *pkg.Image) err
return err
}

return c.client.Set(&memcache.Item{Key: c.KeyPrefix + t.String(), Value: val, Expiration: int32(c.TTL)})
return c.client.Set(&memcache.Item{Key: c.KeyPrefix + t.String(), Value: val, Expiration: int32(c.TTL)}) // #nosec G115 -- max value applied in Initialize
}
2 changes: 1 addition & 1 deletion internal/checks/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const hexBase = 16
const maxColorInt = 0xFFFFFF

func makeImage() (pkg.Image, error) {
col := strconv.FormatUint(rand.Uint64N(maxColorInt), hexBase)
col := strconv.FormatUint(rand.Uint64N(maxColorInt), hexBase) // #nosec G404
if len(col) < numColorDigits {
col = strings.Repeat("0", numColorDigits-len(col)) + col
}
Expand Down
11 changes: 9 additions & 2 deletions internal/images/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"image/png"
"math/rand"
"os"
"path/filepath"
"strings"
)

Expand Down Expand Up @@ -117,6 +118,7 @@ func GetStaticImage(path string) (*[]byte, error) {
}

if failedImages[path] != nil {
//#nosec G404
if rand.Float32()*100 > 1 {
return nil, failedImages[path]
}
Expand All @@ -126,7 +128,7 @@ func GetStaticImage(path string) (*[]byte, error) {
return getColorImage(path)
}

img, err := os.ReadFile(path)
img, err := os.ReadFile(filepath.Clean(path))

if img != nil {
dynamicImages[path] = &img
Expand Down Expand Up @@ -160,7 +162,12 @@ func getColorImage(path string) (*[]byte, error) {
return nil, err
}

writer.Flush()
err = writer.Flush()

if err != nil {
return nil, err
}

output := buf.Bytes()

dynamicImages[path] = &output
Expand Down
14 changes: 12 additions & 2 deletions internal/providers/blend.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,20 @@ func (t Blend) GenerateTile(ctx context.Context, providerContext layer.ProviderC
writer := bufio.NewWriter(&buf)

err := png.Encode(writer, combinedImg)
writer.Flush()

if err != nil {
return nil, err
}

err = writer.Flush()

if err != nil {
return nil, err
}

output := buf.Bytes()

return &pkg.Image{Content: output, ContentType: mimePng, ForceSkipCache: skipWrite.Load()}, err
return &pkg.Image{Content: output, ContentType: mimePng, ForceSkipCache: skipWrite.Load()}, nil
}

func (t Blend) blendImage(ctx context.Context, img image.Image, size image.Point, combinedImg image.Image) image.Image {
Expand Down
14 changes: 12 additions & 2 deletions internal/providers/effect.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,18 @@ func (t Effect) GenerateTile(ctx context.Context, providerContext layer.Provider
writer := bufio.NewWriter(&buf)

err = png.Encode(writer, realImage)
writer.Flush()

if err != nil {
return nil, err
}

err = writer.Flush()

if err != nil {
return nil, err
}

output := buf.Bytes()

return &pkg.Image{Content: output, ContentType: mimePng, ForceSkipCache: img.ForceSkipCache}, err
return &pkg.Image{Content: output, ContentType: mimePng, ForceSkipCache: img.ForceSkipCache}, nil
}
11 changes: 8 additions & 3 deletions internal/providers/postgis_mvt.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"log/slog"
"math"
"regexp"
"slices"
"strconv"
Expand All @@ -37,14 +38,14 @@ var columnRegex = regexp.MustCompile("^[a-zA-Z0-9_]+$")
type PostgisMvtConfig struct {
Datastore string
Table string
Extent uint
Extent uint16
Buffer float64
GID string
Geometry string
Attributes []string
Filter string
SourceSRID uint
Limit uint
Limit uint16
}

type PostgisMvt struct {
Expand Down Expand Up @@ -133,7 +134,11 @@ func (t PostgisMvt) GenerateTile(ctx context.Context, _ layer.ProviderContext, r
rawEnv := bounds.ToEWKT()
bufEnv := bounds.BufferRelative(t.Buffer).ToEWKT()

params := []any{int(t.SourceSRID), rawEnv, t.Extent, int(t.Buffer * float64(t.Extent)), bufEnv, req.LayerName, t.GID}
if t.SourceSRID > math.MaxInt {
return nil, pkg.InvalidSridError{}
}

params := []any{int(t.SourceSRID), rawEnv, t.Extent, int(t.Buffer * float64(t.Extent)), bufEnv, req.LayerName, t.GID} // #nosec G115

query := `WITH mvtgeom AS(SELECT ST_AsMVTGeom(ST_Transform(ST_SetSRID("` + t.Geometry + `", $1::integer), 3857), $2::geometry, extent => $3, buffer => $4) AS "geom"`

Expand Down
14 changes: 12 additions & 2 deletions internal/providers/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ func (t Transform) PreAuth(ctx context.Context, providerContext layer.ProviderCo
return t.provider.PreAuth(ctx, providerContext)
}

// #nosec G115
func (t Transform) transform(ctx context.Context, col color.Color) color.Color {
r1, g1, b1, a1 := col.RGBA()
r1b := uint8(r1)
Expand Down Expand Up @@ -201,8 +202,17 @@ func (t Transform) GenerateTile(ctx context.Context, providerContext layer.Provi
writer := bufio.NewWriter(&buf)

err = png.Encode(writer, resultImage)
writer.Flush()

if err != nil {
return nil, err
}

err = writer.Flush()

if err != nil {
return nil, err
}
output := buf.Bytes()

return &pkg.Image{Content: output, ContentType: mimePng, ForceSkipCache: img.ForceSkipCache}, err
return &pkg.Image{Content: output, ContentType: mimePng, ForceSkipCache: img.ForceSkipCache}, nil
}
2 changes: 1 addition & 1 deletion internal/providers/url_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (t URLTemplate) GenerateTile(ctx context.Context, _ layer.ProviderContext,
url = strings.ReplaceAll(url, "$zoom", strconv.Itoa(tileRequest.Z))
url = strings.ReplaceAll(url, "$width", strconv.Itoa(int(t.Width)))
url = strings.ReplaceAll(url, "$height", strconv.Itoa(int(t.Height)))
url = strings.ReplaceAll(url, "$srs", strconv.Itoa(int(t.Srid)))
url = strings.ReplaceAll(url, "$srs", strconv.FormatUint(uint64(t.Srid), 10))

return getTile(ctx, t.clientConfig, url, make(map[string]string))
}
4 changes: 4 additions & 0 deletions internal/providers/utility.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ func getTile(ctx context.Context, clientConfig config.ClientConfig, url string,
req.Header.Set(h, v)
}

if clientConfig.Timeout > math.MaxInt64 {
clientConfig.Timeout = math.MaxInt64
}

transport := otelhttp.NewTransport(http.DefaultTransport, otelhttp.WithMessageEvents(otelhttp.ReadEvents))
client := http.Client{Transport: transport, Timeout: time.Duration(clientConfig.Timeout) * time.Second}

Expand Down
15 changes: 11 additions & 4 deletions internal/server/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"errors"
"fmt"
"log/slog"
"math"
"net"
"net/http"
"reflect"
Expand Down Expand Up @@ -186,9 +187,10 @@ func setupHealthEndpoints(ctx context.Context, h config.HealthConfig, checks []h
r.Handle("/health", healthHandler{checks, checkResultCache})

srv := &http.Server{
Addr: httpHostPort,
BaseContext: func(_ net.Listener) context.Context { return ctx },
Handler: &r,
Addr: httpHostPort,
BaseContext: func(_ net.Listener) context.Context { return ctx },
Handler: &r,
ReadHeaderTimeout: time.Second,
}

go func() { srvErr <- srv.ListenAndServe() }()
Expand Down Expand Up @@ -219,7 +221,12 @@ func setupCheckRoutines(ctx context.Context, h config.HealthConfig, layerGroup *
}

for i, check := range checks {
ttl := time.Second * time.Duration(check.GetDelay())
delay := check.GetDelay()

if delay > math.MaxInt64 {
delay = math.MaxInt64
}
ttl := time.Second * time.Duration(delay) // #nosec G115

ticker := time.NewTicker(ttl)
done := make(chan bool)
Expand Down
3 changes: 2 additions & 1 deletion internal/server/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"log/slog"
"net/http"
"os"
"path/filepath"
"slices"
"strings"

Expand All @@ -44,7 +45,7 @@ func (h slogContextHandler) Handle(ctx context.Context, r slog.Record) error {
}

func makeLogFileWriter(path string, alsoStdOut bool) (io.Writer, error) {
logFile, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_RDWR, 0666)
logFile, err := os.OpenFile(filepath.Clean(path), os.O_CREATE|os.O_APPEND|os.O_RDWR, 0600)

if err != nil {
return nil, err
Expand Down
30 changes: 24 additions & 6 deletions internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"errors"
"fmt"
"log/slog"
"math"
"net"
"net/http"
"os"
Expand Down Expand Up @@ -98,8 +99,12 @@ func setupHandlers(config *config.Config, layerGroup *layer.LayerGroup, auth aut
rootHandler = handlers.CompressHandler(rootHandler)
}

if config.Server.Timeout > math.MaxInt64 {
config.Server.Timeout = math.MaxInt64
}

rootHandler = httpContextHandler{rootHandler, config.Error}
rootHandler = http.TimeoutHandler(rootHandler, time.Duration(config.Server.Timeout)*time.Second, config.Error.Messages.Timeout)
rootHandler = http.TimeoutHandler(rootHandler, time.Duration(config.Server.Timeout)*time.Second, config.Error.Messages.Timeout) // #nosec G115
rootHandler, err = configureAccessLogging(config.Logging.Access, config.Error.Messages, rootHandler)

if err != nil {
Expand All @@ -115,8 +120,14 @@ func listenAndServeTLS(config *config.Config, srvErr chan error, srv *http.Serve

if config.Server.Encrypt.Certificate != "" && config.Server.Encrypt.KeyFile != "" {
if httpPort != 0 {
srv := &http.Server{
Addr: httpHostPort,
Handler: httpRedirectHandler{protoAndHost: "https://" + config.Server.Encrypt.Domain},
ReadHeaderTimeout: time.Second,
}

go func() {
srvErr <- http.ListenAndServe(httpHostPort, httpRedirectHandler{protoAndHost: "https://" + config.Server.Encrypt.Domain})
srvErr <- srv.ListenAndServe()
}()
}

Expand All @@ -136,7 +147,13 @@ func listenAndServeTLS(config *config.Config, srvErr chan error, srv *http.Serve
}

if httpPort != 0 {
go func() { srvErr <- http.ListenAndServe(httpHostPort, certManager.HTTPHandler(nil)) }()
srv := &http.Server{
Addr: httpHostPort,
Handler: certManager.HTTPHandler(nil),
ReadHeaderTimeout: time.Second,
}

go func() { srvErr <- srv.ListenAndServe() }()
}

srv.TLSConfig = certManager.TLSConfig()
Expand Down Expand Up @@ -186,9 +203,10 @@ func ListenAndServe(config *config.Config, layerGroup *layer.LayerGroup, auth au
}

srv := &http.Server{
Addr: config.Server.BindHost + ":" + strconv.Itoa(config.Server.Port),
BaseContext: func(_ net.Listener) context.Context { return ctx },
Handler: rootHandler,
Addr: config.Server.BindHost + ":" + strconv.Itoa(config.Server.Port),
BaseContext: func(_ net.Listener) context.Context { return ctx },
Handler: rootHandler,
ReadHeaderTimeout: time.Second,
}

srvErr := make(chan error, 1)
Expand Down
Loading