fix(datasets): accept BigTIFF headers and sanitise object keys
F1: ValidateFileContent rejected the pipeline's own output. GDAL writes a
BigTIFF header ("II+\0" / "MM\0+") whenever BIGTIFF=IF_SAFER kicks in for a
large raster, so accept both alongside the classic TIFF magic and reword the
error to "file is not a valid TIFF/BigTIFF GeoTIFF". GodalIO.Create now
delegates to an unexported createGTiff so a test can force BIGTIFF=YES and
assert the produced header passes the check.
F3: a user-supplied filename went straight into the S3 object key for dataset
uploads and for both dataset and dataset-group SLD uploads, so "../../x.sld"
escaped the dataset's own prefix. Reduce every such name to its base with a
shared safeObjectName helper, rejecting names with no usable base.
F9: rename the shadowing inner "raw" in the PATCH dg_id case to "s".
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -127,9 +127,23 @@ func ExtensionAllowedFor(ft FileType, ext string) bool {
|
|||||||
func ValidateFileContent(ext string, head []byte) error {
|
func ValidateFileContent(ext string, head []byte) error {
|
||||||
switch ext {
|
switch ext {
|
||||||
case ".tif", ".tiff":
|
case ".tif", ".tiff":
|
||||||
// TIFF: little-endian "II*\0" or big-endian "MM\0*".
|
// Classic TIFF: little-endian "II*\0" or big-endian "MM\0*".
|
||||||
if !bytes.HasPrefix(head, []byte("II*\x00")) && !bytes.HasPrefix(head, []byte("MM\x00*")) {
|
// BigTIFF (version 43) uses "II+\0" / "MM\0+"; GDAL writes it for
|
||||||
return fmt.Errorf("file is not a valid TIFF/GeoTIFF")
|
// large rasters (BIGTIFF=IF_SAFER), so the pipeline's own COG output
|
||||||
|
// must pass this check too.
|
||||||
|
tiffMagic := [][]byte{
|
||||||
|
[]byte("II*\x00"), []byte("MM\x00*"),
|
||||||
|
[]byte("II+\x00"), []byte("MM\x00+"),
|
||||||
|
}
|
||||||
|
ok := false
|
||||||
|
for _, m := range tiffMagic {
|
||||||
|
if bytes.HasPrefix(head, m) {
|
||||||
|
ok = true
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !ok {
|
||||||
|
return fmt.Errorf("file is not a valid TIFF/BigTIFF GeoTIFF")
|
||||||
}
|
}
|
||||||
case ".zip":
|
case ".zip":
|
||||||
// ZIP local-file or empty-archive signature.
|
// ZIP local-file or empty-archive signature.
|
||||||
|
|||||||
@@ -30,7 +30,10 @@ func TestValidateFileContent(t *testing.T) {
|
|||||||
}{
|
}{
|
||||||
{"valid tiff LE", ".tif", []byte("II*\x00rest"), true},
|
{"valid tiff LE", ".tif", []byte("II*\x00rest"), true},
|
||||||
{"valid tiff BE", ".tiff", []byte("MM\x00*rest"), true},
|
{"valid tiff BE", ".tiff", []byte("MM\x00*rest"), true},
|
||||||
|
{"valid bigtiff LE", ".tif", []byte("II+\x00rest"), true},
|
||||||
|
{"valid bigtiff BE", ".tiff", []byte("MM\x00+rest"), true},
|
||||||
{"bad tiff", ".tif", []byte("not a tiff"), false},
|
{"bad tiff", ".tif", []byte("not a tiff"), false},
|
||||||
|
{"bad tiff near-miss prefix", ".tif", []byte("II-\x00rest"), false},
|
||||||
{"valid zip", ".zip", []byte("PK\x03\x04rest"), true},
|
{"valid zip", ".zip", []byte("PK\x03\x04rest"), true},
|
||||||
{"bad zip", ".zip", []byte("RARrest"), false},
|
{"bad zip", ".zip", []byte("RARrest"), false},
|
||||||
{"valid gpkg", ".gpkg", []byte("SQLite format 3\x00rest"), true},
|
{"valid gpkg", ".gpkg", []byte("SQLite format 3\x00rest"), true},
|
||||||
|
|||||||
@@ -66,10 +66,17 @@ func (r *godalReader) Close() error { return r.ds.Close() }
|
|||||||
type godalWriter struct{ ds *godal.Dataset }
|
type godalWriter struct{ ds *godal.Dataset }
|
||||||
|
|
||||||
func (g *GodalIO) Create(_ context.Context, path string, grid Grid, o WriterOptions) (Writer, error) {
|
func (g *GodalIO) Create(_ context.Context, path string, grid Grid, o WriterOptions) (Writer, error) {
|
||||||
|
return createGTiff(path, grid, o, "IF_SAFER")
|
||||||
|
}
|
||||||
|
|
||||||
|
// createGTiff writes a tiled Int16 GeoTIFF. bigTIFF is the value of the
|
||||||
|
// BIGTIFF creation option ("IF_SAFER" in production); it is a parameter so
|
||||||
|
// tests can force a BigTIFF header.
|
||||||
|
func createGTiff(path string, grid Grid, o WriterOptions, bigTIFF string) (Writer, error) {
|
||||||
tile := fmt.Sprint(o.TileSize)
|
tile := fmt.Sprint(o.TileSize)
|
||||||
ds, err := godal.Create(godal.GTiff, path, 1, godal.Int16, grid.Width, grid.Height,
|
ds, err := godal.Create(godal.GTiff, path, 1, godal.Int16, grid.Width, grid.Height,
|
||||||
godal.CreationOption("TILED=YES", "BLOCKXSIZE="+tile, "BLOCKYSIZE="+tile,
|
godal.CreationOption("TILED=YES", "BLOCKXSIZE="+tile, "BLOCKYSIZE="+tile,
|
||||||
"COMPRESS="+o.Compression, "PREDICTOR=2", "BIGTIFF=IF_SAFER"))
|
"COMPRESS="+o.Compression, "PREDICTOR=2", "BIGTIFF="+bigTIFF))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("create raster %s: %w", path, err)
|
return nil, fmt.Errorf("create raster %s: %w", path, err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,9 +1,14 @@
|
|||||||
package raster
|
package raster
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
|
"io"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"gis/internal/domain"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestGodalIO_RoundTrip(t *testing.T) {
|
func TestGodalIO_RoundTrip(t *testing.T) {
|
||||||
@@ -60,3 +65,39 @@ func TestGodalIO_RoundTrip(t *testing.T) {
|
|||||||
t.Fatal("SameGrid should be false for shifted origin")
|
t.Fatal("SameGrid should be false for shifted origin")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestGodalIO_BigTIFFPassesContentValidation guards the upload magic-byte
|
||||||
|
// check against the pipeline's own output: GDAL writes a BigTIFF header
|
||||||
|
// (version 43) whenever BIGTIFF is YES or IF_SAFER kicks in for a large
|
||||||
|
// raster, and domain.ValidateFileContent must accept it.
|
||||||
|
func TestGodalIO_BigTIFFPassesContentValidation(t *testing.T) {
|
||||||
|
path := filepath.Join(t.TempDir(), "big.tif")
|
||||||
|
grid := Grid{Width: 8, Height: 8, GeoTransform: [6]float64{100, 30, 0, 200, 0, -30}, ProjectionWKT: "EPSG:32642"}
|
||||||
|
|
||||||
|
w, err := createGTiff(path, grid, DefaultWriterOptions(), "YES")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := w.WriteInt16(Window{0, 0, 8, 8}, make([]int16, 8*8)); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := w.Close(); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
f, err := os.Open(path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
defer f.Close()
|
||||||
|
head := make([]byte, 4)
|
||||||
|
if _, err := io.ReadFull(f, head); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if !bytes.HasPrefix(head, []byte("II+\x00")) && !bytes.HasPrefix(head, []byte("MM\x00+")) {
|
||||||
|
t.Fatalf("BIGTIFF=YES did not produce a BigTIFF header: % x", head)
|
||||||
|
}
|
||||||
|
if err := domain.ValidateFileContent(".tif", head); err != nil {
|
||||||
|
t.Fatalf("ValidateFileContent rejected our own BigTIFF output: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -226,7 +226,11 @@ func (s *DatasetService) Upload(ctx context.Context, in UploadInput) (domain.Dat
|
|||||||
status = domain.DatasetStatusParsing
|
status = domain.DatasetStatusParsing
|
||||||
}
|
}
|
||||||
|
|
||||||
storageKey := fmt.Sprintf("%s/%s", uuid.New().String(), in.Filename)
|
objectName, err := safeObjectName(in.Filename)
|
||||||
|
if err != nil {
|
||||||
|
return domain.Dataset{}, err
|
||||||
|
}
|
||||||
|
storageKey := fmt.Sprintf("%s/%s", uuid.New().String(), objectName)
|
||||||
if err := s.store.Put(ctx, storageKey, content, in.Size, in.ContentType); err != nil {
|
if err := s.store.Put(ctx, storageKey, content, in.Size, in.ContentType); err != nil {
|
||||||
return domain.Dataset{}, err
|
return domain.Dataset{}, err
|
||||||
}
|
}
|
||||||
@@ -297,7 +301,10 @@ func (s *DatasetService) UploadSLD(ctx context.Context, id uuid.UUID, in UploadS
|
|||||||
}
|
}
|
||||||
content := io.MultiReader(bytes.NewReader(head), in.Reader)
|
content := io.MultiReader(bytes.NewReader(head), in.Reader)
|
||||||
|
|
||||||
key := deriveSLDKey(dataset.StorageKey, in.Filename)
|
key, err := deriveSLDKey(dataset.StorageKey, in.Filename)
|
||||||
|
if err != nil {
|
||||||
|
return domain.Dataset{}, err
|
||||||
|
}
|
||||||
if err := s.store.Put(ctx, key, content, in.Size, in.ContentType); err != nil {
|
if err := s.store.Put(ctx, key, content, in.Size, in.ContentType); err != nil {
|
||||||
return domain.Dataset{}, err
|
return domain.Dataset{}, err
|
||||||
}
|
}
|
||||||
@@ -391,10 +398,27 @@ func (s *DatasetService) CreateRasterFromFile(ctx context.Context, in CreateRast
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// safeObjectName reduces a user-supplied filename to a single path element so
|
||||||
|
// it can never escape the object-key prefix it is appended to. Rejects names
|
||||||
|
// that carry no usable base ("", ".", "..", "/").
|
||||||
|
func safeObjectName(name string) (string, error) {
|
||||||
|
base := path.Base(filepath.ToSlash(strings.TrimSpace(name)))
|
||||||
|
switch base {
|
||||||
|
case "", ".", "..", "/":
|
||||||
|
return "", fmt.Errorf("%w: invalid filename", domain.ErrValidation)
|
||||||
|
}
|
||||||
|
return base, nil
|
||||||
|
}
|
||||||
|
|
||||||
// deriveSLDKey places a dataset's SLD style file alongside the original under
|
// deriveSLDKey places a dataset's SLD style file alongside the original under
|
||||||
// an sld/ prefix, named after the uploaded SLD file.
|
// an sld/ prefix, named after the uploaded SLD file. The filename is
|
||||||
func deriveSLDKey(storageKey, filename string) string {
|
// sanitised, so the key always stays under the dataset's own prefix.
|
||||||
return path.Join(path.Dir(storageKey), "sld", filename)
|
func deriveSLDKey(storageKey, filename string) (string, error) {
|
||||||
|
name, err := safeObjectName(filename)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
return path.Join(path.Dir(storageKey), "sld", name), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// enqueueProcessing schedules the appropriate async job for a dataset based on
|
// enqueueProcessing schedules the appropriate async job for a dataset based on
|
||||||
|
|||||||
@@ -151,7 +151,11 @@ func (s *DatasetGroupService) UploadSLD(ctx context.Context, id uuid.UUID, in Up
|
|||||||
}
|
}
|
||||||
content := io.MultiReader(bytes.NewReader(head), in.Reader)
|
content := io.MultiReader(bytes.NewReader(head), in.Reader)
|
||||||
|
|
||||||
key := path.Join("groups", g.ID.String(), "sld", in.Filename)
|
name, err := safeObjectName(in.Filename)
|
||||||
|
if err != nil {
|
||||||
|
return domain.DatasetGroup{}, err
|
||||||
|
}
|
||||||
|
key := path.Join("groups", g.ID.String(), "sld", name)
|
||||||
if err := s.store.Put(ctx, key, content, in.Size, in.ContentType); err != nil {
|
if err := s.store.Put(ctx, key, content, in.Size, in.ContentType); err != nil {
|
||||||
return domain.DatasetGroup{}, err
|
return domain.DatasetGroup{}, err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -255,3 +255,29 @@ func TestDatasetGroupService_Update_RequiresSlug(t *testing.T) {
|
|||||||
t.Fatalf("want ErrValidation, got %v", err)
|
t.Fatalf("want ErrValidation, got %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestDatasetGroupService_UploadSLD_RejectsPathTraversal(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
groups := newStubGroupRepo()
|
||||||
|
store := newStubStore()
|
||||||
|
svc := NewDatasetGroupService(groups, newStubDatasetRepo(), store)
|
||||||
|
g, err := svc.Create(ctx, DatasetGroupInput{Code: "pop", Name: "Population"})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
updated, err := svc.UploadSLD(ctx, g.ID, UploadSLDInput{
|
||||||
|
Filename: "../../other/sld/evil.sld", ContentType: "application/xml", Size: 3, Reader: strings.NewReader("<x>"),
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
wantPrefix := "groups/" + g.ID.String() + "/sld/"
|
||||||
|
if updated.SldStorageKey == nil || !strings.HasPrefix(*updated.SldStorageKey, wantPrefix) ||
|
||||||
|
!strings.HasSuffix(*updated.SldStorageKey, "evil.sld") {
|
||||||
|
t.Fatalf("sld key escaped the group prefix: %v", updated.SldStorageKey)
|
||||||
|
}
|
||||||
|
if len(store.put) != 1 || !strings.HasPrefix(store.put[0], wantPrefix) {
|
||||||
|
t.Fatalf("object stored outside the group sld prefix: %v", store.put)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -1926,3 +1926,86 @@ func TestDatasetService_CreateRasterFromFile_MissingFile(t *testing.T) {
|
|||||||
t.Fatal("want an error for a missing file")
|
t.Fatal("want an error for a missing file")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSafeObjectName(t *testing.T) {
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
in string
|
||||||
|
want string
|
||||||
|
wantErr bool
|
||||||
|
}{
|
||||||
|
{"plain name", "style.sld", "style.sld", false},
|
||||||
|
{"trims space", " style.sld ", "style.sld", false},
|
||||||
|
{"nested path reduced to base", "a/b/style.sld", "style.sld", false},
|
||||||
|
{"traversal reduced to base", "../../x.sld", "x.sld", false},
|
||||||
|
{"absolute path reduced to base", "/etc/passwd", "passwd", false},
|
||||||
|
{"dotdot alone", "..", "", true},
|
||||||
|
{"dot alone", ".", "", true},
|
||||||
|
{"root alone", "/", "", true},
|
||||||
|
{"empty", "", "", true},
|
||||||
|
{"blank", " ", "", true},
|
||||||
|
{"trailing slash", "a/b/", "b", false},
|
||||||
|
}
|
||||||
|
for _, c := range cases {
|
||||||
|
t.Run(c.name, func(t *testing.T) {
|
||||||
|
got, err := safeObjectName(c.in)
|
||||||
|
if c.wantErr {
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("want error, got %q", got)
|
||||||
|
}
|
||||||
|
if !errors.Is(err, domain.ErrValidation) {
|
||||||
|
t.Fatalf("want ErrValidation, got %v", err)
|
||||||
|
}
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if got != c.want {
|
||||||
|
t.Fatalf("safeObjectName(%q) = %q, want %q", c.in, got, c.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestDatasetService_UploadSLD_RejectsPathTraversal(t *testing.T) {
|
||||||
|
id := uuid.New()
|
||||||
|
repo := newStubDatasetRepo()
|
||||||
|
repo.store[id] = domain.Dataset{ID: id, FileType: domain.FileTypeRaster, StorageKey: "uid/dem.tif"}
|
||||||
|
store := &stubStore{}
|
||||||
|
svc := newDatasetService(repo, store, true)
|
||||||
|
|
||||||
|
in := validSLDUpload()
|
||||||
|
in.Filename = "../../other/sld/evil.sld"
|
||||||
|
got, err := svc.UploadSLD(context.Background(), id, in)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if got.SldStorageKey == nil || *got.SldStorageKey != "uid/sld/evil.sld" {
|
||||||
|
t.Fatalf("sld key escaped the dataset prefix: %v", got.SldStorageKey)
|
||||||
|
}
|
||||||
|
if len(store.put) != 1 || !strings.HasPrefix(store.put[0], "uid/sld/") || !strings.HasSuffix(store.put[0], "evil.sld") {
|
||||||
|
t.Fatalf("object stored outside the dataset sld prefix: %v", store.put)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestDatasetService_Upload_SanitisesTraversalFilename(t *testing.T) {
|
||||||
|
repo := newStubDatasetRepo()
|
||||||
|
store := &stubStore{}
|
||||||
|
svc := newDatasetService(repo, store, true)
|
||||||
|
|
||||||
|
in := validUpload()
|
||||||
|
in.Filename = "../../evil.geojson"
|
||||||
|
got, err := svc.Upload(context.Background(), in)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
// The key is "<uuid>/<basename>": exactly two segments, no traversal.
|
||||||
|
parts := strings.Split(got.StorageKey, "/")
|
||||||
|
if len(parts) != 2 || parts[1] != "evil.geojson" {
|
||||||
|
t.Fatalf("storage key escaped its uuid prefix: %q", got.StorageKey)
|
||||||
|
}
|
||||||
|
if len(store.put) != 1 || store.put[0] != got.StorageKey {
|
||||||
|
t.Fatalf("object stored under an unexpected key: %v", store.put)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -481,13 +481,13 @@ func (h *DatasetHandler) patch(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
in.Unit, in.UnitSet = unit, true
|
in.Unit, in.UnitSet = unit, true
|
||||||
case "dg_id":
|
case "dg_id":
|
||||||
raw, err := decodeOptionalPatchString(val)
|
s, err := decodeOptionalPatchString(val)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
httputil.WriteError(w, http.StatusUnprocessableEntity, "dg_id must be a valid UUID or null")
|
httputil.WriteError(w, http.StatusUnprocessableEntity, "dg_id must be a valid UUID or null")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if raw != nil {
|
if s != nil {
|
||||||
gid, err := uuid.Parse(*raw)
|
gid, err := uuid.Parse(*s)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
httputil.WriteError(w, http.StatusUnprocessableEntity, "dg_id must be a valid UUID or null")
|
httputil.WriteError(w, http.StatusUnprocessableEntity, "dg_id must be a valid UUID or null")
|
||||||
return
|
return
|
||||||
|
|||||||
Reference in New Issue
Block a user