Skip to content

Commit

Permalink
Merge pull request #254 from Michad/gosec
Browse files Browse the repository at this point in the history
security: fix issues reported by new version of gosec
  • Loading branch information
Michad authored Sep 28, 2024
2 parents 7cbc8c7 + 4af8dbe commit 4501370
Show file tree
Hide file tree
Showing 20 changed files with 132 additions and 43 deletions.
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

0 comments on commit 4501370

Please sign in to comment.