Skip to content

fix: path traversal #300

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.

Sign up for GitHub

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

Open
InvalidJoker wants to merge 2 commits into
base: main
Choose a base branch
from
+211 −4
Open

fix: path traversal #300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df880cf
fix: path traversal
InvalidJoker May 4, 2026
c6dbac7
no need to check old data
InvalidJoker May 10, 2026
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
24 changes: 21 additions & 3 deletions controlplane/chunk/flavor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"errors"
"fmt"
"maps"
"path/filepath"
"slices"
"sort"
"strings"
Expand Down Expand Up @@ -129,6 +130,10 @@ func (s *svc) CreateFlavorVersion(
fmt.Errorf("minecraft version exists: %w", err)
}

if err := cleanFileHashPaths(version.FileHashes); err != nil {
return resource.FlavorVersion{}, resource.FlavorVersionDiff{}, err
}

prevVersion, err := s.repo.LatestFlavorVersion(ctx, flavorID)
if err != nil {
return resource.FlavorVersion{},
Expand All @@ -145,9 +150,6 @@ func (s *svc) CreateFlavorVersion(
return resource.FlavorVersion{}, resource.FlavorVersionDiff{}, apierrs.ErrHashMismatch
}

// TODO: clean all received paths using filepath.Clean to avoid
// possible path traversal techniques. relative paths are not allowed.

var (
unchanged = make([]file.Hash, 0)
changed = make([]file.Hash, 0)
Expand Down Expand Up @@ -237,6 +239,22 @@ func (s *svc) CreateFlavorVersion(
return created, diff, nil
}

func cleanFileHashPaths(hashes []file.Hash) error {
for i := range hashes {
cleanPath := filepath.Clean(hashes[i].Path)
if cleanPath == "." ||
filepath.IsAbs(cleanPath) ||
cleanPath == ".." ||
strings.HasPrefix(cleanPath, ".."+string(filepath.Separator)) {
return apierrs.ErrInvalidPath
}

hashes[i].Path = filepath.ToSlash(cleanPath)
}

return nil
}

func (s *svc) BuildFlavorVersion(ctx context.Context, versionID string) error {
actorID, ok := ctx.Value(contextkey.ActorID).(string)
if !ok {
Expand Down
190 changes: 189 additions & 1 deletion controlplane/chunk/flavor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,45 @@ func TestCreateFlavor(t *testing.T) {
}

func TestCreateFlavorVersion(t *testing.T) {
cleanedPathVersion := fixture.FlavorVersion(func(v *resource.FlavorVersion) {
v.Version = "v2"
v.FileHashes = []file.Hash{
{
Path: "paper.yml",
Hash: "pppppppppppppppp",
},
{
Path: "server.properties",
Hash: "hash changed",
},
{
Path: "plugins/myplugin.jar",
Hash: "hash1",
},
}
v.ChangeHash = "68df46974f6dc5fe"
})
uncleanPathVersion := cleanedPathVersion
uncleanPathVersion.FileHashes = []file.Hash{
{
Path: "paper.yml",
Hash: "pppppppppppppppp",
},
{
Path: "plugins/myplugin.jar",
Hash: "hash1",
},
{
Path: "plugins/../server.properties",
Hash: "hash changed",
},
}

tests := []struct {
name string
prevVersion resource.FlavorVersion
newVersion resource.FlavorVersion
expected *resource.FlavorVersion
expectedDiff resource.FlavorVersionDiff
prep func(
*mock.MockChunkRepository,
Expand Down Expand Up @@ -213,6 +248,155 @@ func TestCreateFlavorVersion(t *testing.T) {
Return(newVersion, nil)
},
},
{
name: "cleans paths",
prevVersion: fixture.FlavorVersion(),
newVersion: uncleanPathVersion,
expected: &cleanedPathVersion,
expectedDiff: resource.FlavorVersionDiff{
Added: []file.Hash{
{
Path: "plugins/myplugin.jar",
Hash: "hash1",
},
},
Removed: []file.Hash{
{
Path: "plugins/myplugin/config.json",
Hash: "cooooooooooooooo",
},
},
Changed: []file.Hash{
{
Path: "server.properties",
Hash: "hash changed",
},
},
},
prep: func(
repo *mock.MockChunkRepository,
newVersion resource.FlavorVersion,
prevVersion resource.FlavorVersion,
access *mock.MockAuthzAccessEvaluator,
) {
access.EXPECT().
AccessAuthorized(
mocky.Anything,
mocky.AnythingOfType("authz.AccessRuleOption"),
).
Return(nil)

repo.EXPECT().
FlavorByID(mocky.Anything, fixture.Flavor().ID).
Return(fixture.Flavor(), nil)

repo.EXPECT().
FlavorVersionExists(mocky.Anything, fixture.Flavor().ID, newVersion.Version).
Return(false, nil)

repo.EXPECT().
GetMinecraftVersionByVersion(mocky.Anything, newVersion.MinecraftVersion).
Return(resource.MinecraftVersion{
Version: newVersion.MinecraftVersion,
ImageURL: "some-url",
CreatedAt: time.Time{},
}, nil)

repo.EXPECT().
LatestFlavorVersion(mocky.Anything, fixture.Flavor().ID).
Return(prevVersion, nil)

repo.EXPECT().
CreateFlavorVersion(mocky.Anything, fixture.FlavorID, cleanedPathVersion, prevVersion.ID).
Return(cleanedPathVersion, nil)
},
},
{
name: "rejects relative traversal paths",
prevVersion: fixture.FlavorVersion(),
newVersion: fixture.FlavorVersion(func(v *resource.FlavorVersion) {
v.Version = "v2"
v.FileHashes = []file.Hash{
{
Path: "../server.properties",
Hash: "hash changed",
},
}
}),
prep: func(
repo *mock.MockChunkRepository,
newVersion resource.FlavorVersion,
prevVersion resource.FlavorVersion,
access *mock.MockAuthzAccessEvaluator,
) {
access.EXPECT().
AccessAuthorized(
mocky.Anything,
mocky.AnythingOfType("authz.AccessRuleOption"),
).
Return(nil)

repo.EXPECT().
FlavorByID(mocky.Anything, fixture.Flavor().ID).
Return(fixture.Flavor(), nil)

repo.EXPECT().
FlavorVersionExists(mocky.Anything, fixture.Flavor().ID, newVersion.Version).
Return(false, nil)

repo.EXPECT().
GetMinecraftVersionByVersion(mocky.Anything, newVersion.MinecraftVersion).
Return(resource.MinecraftVersion{
Version: newVersion.MinecraftVersion,
ImageURL: "some-url",
CreatedAt: time.Time{},
}, nil)
},
err: apierrs.ErrInvalidPath,
},
{
name: "rejects absolute paths",
prevVersion: fixture.FlavorVersion(),
newVersion: fixture.FlavorVersion(func(v *resource.FlavorVersion) {
v.Version = "v2"
v.FileHashes = []file.Hash{
{
Path: "/server.properties",
Hash: "hash changed",
},
}
}),
prep: func(
repo *mock.MockChunkRepository,
newVersion resource.FlavorVersion,
prevVersion resource.FlavorVersion,
access *mock.MockAuthzAccessEvaluator,
) {
access.EXPECT().
AccessAuthorized(
mocky.Anything,
mocky.AnythingOfType("authz.AccessRuleOption"),
).
Return(nil)

repo.EXPECT().
FlavorByID(mocky.Anything, fixture.Flavor().ID).
Return(fixture.Flavor(), nil)

repo.EXPECT().
FlavorVersionExists(mocky.Anything, fixture.Flavor().ID, newVersion.Version).
Return(false, nil)

repo.EXPECT().
GetMinecraftVersionByVersion(mocky.Anything, newVersion.MinecraftVersion).
Return(resource.MinecraftVersion{
Version: newVersion.MinecraftVersion,
ImageURL: "some-url",
CreatedAt: time.Time{},
}, nil)
},
err: apierrs.ErrInvalidPath,
},
{
name: "version hash mismatch",
prevVersion: fixture.FlavorVersion(),
Expand Down Expand Up @@ -379,7 +563,11 @@ func TestCreateFlavorVersion(t *testing.T) {
}

require.NoError(t, err)
require.Equal(t, tt.newVersion, actualNewVersion)
expected := tt.newVersion
if tt.expected != nil {
expected = *tt.expected
}
require.Equal(t, expected, actualNewVersion)
require.Equal(t, tt.expectedDiff, actualDiff)
})
}
Expand Down
1 change: 1 addition & 0 deletions controlplane/errors/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ var (
ErrMinecraftVersionNotSupported = New(codes.FailedPrecondition, "minecraft version not found")
ErrHashMismatch = New(codes.FailedPrecondition, "hash does not match")
ErrInvalidHash = New(codes.InvalidArgument, "invalid hash")
ErrInvalidPath = New(codes.InvalidArgument, "invalid path")
ErrFlavorFilesNotUploaded = New(codes.FailedPrecondition, "flavor files have not been uploaded")
ErrFlavorFilesUploaded = New(codes.AlreadyExists, "flavor files have already been uploaded")
ErrFlavorVersionNotFound = New(codes.NotFound, "flavor version does not exist")
Expand Down
Loading