Skip to content

Commit

Permalink
Fix "uncontrolled data use" from not verifying input to archivista ge…
Browse files Browse the repository at this point in the history
…t/store request parameters

Signed-off-by: John Kjell <[email protected]>
  • Loading branch information
jkjell committed Jan 10, 2024
1 parent 93977fd commit 44956aa
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 2 deletions.
12 changes: 10 additions & 2 deletions internal/objectstorage/filestore/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,17 @@ func New(ctx context.Context, directory string, address string) (*Store, <-chan
}

func (s *Store) Get(ctx context.Context, gitoid string) (io.ReadCloser, error) {
return os.Open(filepath.Join(s.prefix, gitoid+".json"))
if filepath.IsLocal(gitoid) {
return os.Open(filepath.Join(s.prefix, gitoid+".json"))

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
} else {
return nil, filepath.ErrBadPattern
}
}

func (s *Store) Store(ctx context.Context, gitoid string, payload []byte) error {
return os.WriteFile(filepath.Join(s.prefix, gitoid+".json"), payload, 0644)
if filepath.IsLocal(gitoid) {
return os.WriteFile(filepath.Join(s.prefix, gitoid+".json"), payload, 0644)
} else {
return filepath.ErrBadPattern
}
}
79 changes: 79 additions & 0 deletions internal/objectstorage/filestore/file_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright 2022 The Archivista Contributors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package filestore_test

import (
"context"
"io"
"os"
"path/filepath"
"testing"

filestore "github.com/in-toto/archivista/internal/objectstorage/filestore"
)

func TestStore_Get(t *testing.T) {
// Create a temporary directory for testing
tempDir, err := os.MkdirTemp("", "filestore_test")
if err != nil {
t.Fatalf("Failed to create temporary directory: %v", err)
}
defer os.RemoveAll(tempDir)

// Create a new file store
store, _, err := filestore.New(context.Background(), tempDir, "")
if err != nil {
t.Fatalf("Failed to create file store: %v", err)
}

// Define a test payload
payload := []byte("test payload")

// Store the payload
err = store.Store(context.Background(), "test_gitoid", payload)
if err != nil {
t.Fatalf("Failed to store payload: %v", err)
}

// Attempt storing at malicious payload location
err = store.Store(context.Background(), "../../test_gitoid", payload)
if err != nil && err != filepath.ErrBadPattern {
t.Errorf("Failed to detect bad path: %v", err)
}

// Retrieve the payload
reader, err := store.Get(context.Background(), "test_gitoid")
if err != nil {
t.Errorf("Failed to retrieve payload: %v", err)
}
defer reader.Close()

// Read the payload from the reader
retrievedPayload, err := io.ReadAll(reader)
if err != nil {
t.Fatalf("Failed to read payload: %v", err)
}

// Compare the retrieved payload with the original payload
if string(retrievedPayload) != string(payload) {
t.Errorf("Retrieved payload does not match original payload")
}

// Attempt to retrieve non-local payload
_, err = store.Get(context.Background(), "/etc/passwd")
if err != nil && err != filepath.ErrBadPattern {
t.Errorf("Failed to detect bad path: %v", err)
}

}

0 comments on commit 44956aa

Please sign in to comment.