fix: prevent passing negative expiration during secret creation

Signed-off-by: Knut Ahlers <knut@ahlers.me>

Author: Luzifer

api.go

@@ -16,11 +16,14 @@ import (
 )
 
 const (
+	errorReasonInvalidExpiry  = "invalid_expiry"
 	errorReasonInvalidJSON    = "invalid_json"
 	errorReasonSecretMissing  = "secret_missing"
+	errorReasonSecretNotFound = "secret_not_found"
 	errorReasonSecretSize     = "secret_size"
 	errorReasonStorageError   = "storage_error"
-	errorReasonSecretNotFound = "secret_not_found"
+
+	maxExpirySeconds = int64(1<<63-1) / int64(time.Second)
 )
 
 type apiServer struct {
@@ -69,8 +72,11 @@ func (a apiServer) handleCreate(res http.ResponseWriter, r *http.Request) {
 	)
 
 	if !cust.DisableExpiryOverride {
-		if ev, err := strconv.ParseInt(r.URL.Query().Get("expire"), 10, 64); err == nil && (ev < expiry || cfg.SecretExpiry == 0) {
-			expiry = ev
+		var err error
+		if expiry, err = a.parseExpiryOverride(r, expiry); err != nil {
+			a.collector.CountSecretCreateError(errorReasonInvalidExpiry)
+			a.errorResponse(res, http.StatusBadRequest, err, "")
+			return
 		}
 	}
 
@@ -183,3 +189,29 @@ func (apiServer) jsonResponse(res http.ResponseWriter, status int, response any)
 		http.Error(res, `{"error":"could not encode response"}`, http.StatusInternalServerError)
 	}
 }
+
+func (apiServer) parseExpiryOverride(r *http.Request, expiry int64) (int64, error) {
+	expiryValues, ok := r.URL.Query()["expire"]
+	if !ok {
+		return expiry, nil
+	}
+
+	ev, err := strconv.ParseInt(expiryValues[0], 10, 64)
+	if err != nil {
+		return 0, errors.New("invalid expiry")
+	}
+
+	if ev < 0 {
+		return 0, errors.New("expiry must be greater than or equal to zero")
+	}
+
+	if ev > maxExpirySeconds {
+		return 0, errors.New("expiry exceeds maximum duration")
+	}
+
+	if ev < expiry || cfg.SecretExpiry == 0 {
+		return ev, nil
+	}
+
+	return expiry, nil
+}

api_test.go

@@ -0,0 +1,144 @@
+package main
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/Luzifer/ots/pkg/customization"
+	"github.com/Luzifer/ots/pkg/metrics"
+	"github.com/Luzifer/ots/pkg/storage"
+	"github.com/Luzifer/ots/pkg/storage/memory"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+var testCollector = metrics.New()
+
+func TestHandleCreateExpiryOverrideAcceptedValues(t *testing.T) {
+	tests := []struct {
+		name          string
+		expire        int64
+		wantExpiresAt bool
+	}{
+		{
+			name:          "zero",
+			expire:        0,
+			wantExpiresAt: false,
+		},
+		{
+			name:          "one-second",
+			expire:        1,
+			wantExpiresAt: true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			api, _ := newTestAPI(t)
+			res := createJSONSecret(api, fmt.Sprintf("/api/create?expire=%d", tc.expire))
+
+			require.Equal(t, http.StatusCreated, res.Code)
+
+			var response apiResponse
+			require.NoError(t, json.NewDecoder(res.Body).Decode(&response))
+			assert.True(t, response.Success)
+			assert.NotEmpty(t, response.SecretID)
+			if tc.wantExpiresAt {
+				assert.NotNil(t, response.ExpiresAt)
+			} else {
+				assert.Nil(t, response.ExpiresAt)
+			}
+		})
+	}
+}
+
+func TestHandleCreateExpiryOverrideValidation(t *testing.T) {
+	tests := []struct {
+		name        string
+		contentType string
+		body        string
+		expire      string
+	}{
+		{
+			name:        "empty",
+			contentType: "application/json",
+			body:        `{"secret":"test-secret"}`,
+			expire:      "",
+		},
+		{
+			name:        "malformed",
+			contentType: "application/json",
+			body:        `{"secret":"test-secret"}`,
+			expire:      "abc",
+		},
+		{
+			name:        "negative-json",
+			contentType: "application/json",
+			body:        `{"secret":"test-secret"}`,
+			expire:      "-1",
+		},
+		{
+			name:        "negative-form",
+			contentType: "application/x-www-form-urlencoded",
+			body:        "secret=test-secret",
+			expire:      "-1",
+		},
+		{
+			name:        "too-large",
+			contentType: "application/json",
+			body:        `{"secret":"test-secret"}`,
+			expire:      strconv.FormatInt(maxExpirySeconds+1, 10),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			api, store := newTestAPI(t)
+			req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/api/create?expire="+tc.expire, strings.NewReader(tc.body))
+			req.Header.Set("Content-Type", tc.contentType)
+			res := httptest.NewRecorder()
+
+			api.handleCreate(res, req)
+
+			require.Equal(t, http.StatusBadRequest, res.Code)
+
+			count, err := store.Count()
+			require.NoError(t, err)
+			assert.Zero(t, count)
+		})
+	}
+}
+
+func createJSONSecret(api *apiServer, target string) *httptest.ResponseRecorder {
+	req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, target, bytes.NewBufferString(`{"secret":"test-secret"}`))
+	req.Header.Set("Content-Type", "application/json")
+
+	res := httptest.NewRecorder()
+	api.handleCreate(res, req)
+
+	return res
+}
+
+func newTestAPI(t *testing.T) (*apiServer, storage.Storage) {
+	t.Helper()
+
+	oldCfg := cfg
+	oldCust := cust
+	t.Cleanup(func() {
+		cfg = oldCfg
+		cust = oldCust
+	})
+
+	cfg.SecretExpiry = 3600
+	cust = customization.Customize{}
+
+	store := memory.New()
+	return newAPI(store, testCollector), store
+}