Skip to content

Commit

Permalink
Fixes SAST issues
Browse files Browse the repository at this point in the history
Co-authored-by: joe webster <websterj@netapp.com>
  • Loading branch information
reederc42 and jwebster7 authored Feb 27, 2024
1 parent a2930d4 commit 8059fde
Show file tree
Hide file tree
Showing 14 changed files with 234 additions and 50 deletions.
4 changes: 4 additions & 0 deletions frontend/docker/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"crypto/tls"
"encoding/json"
"fmt"
"math"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -289,6 +290,9 @@ func (p *Plugin) Create(request *volume.CreateRequest) error {
sizeBytes, err := utils.GetVolumeSizeBytes(ctx, request.Options, "0")
if err != nil {
return fmt.Errorf("error creating volume: %v", err)
} else if sizeBytes > math.MaxInt64 {
Logc(ctx).WithFields(fields).Error("Invalid volume size")
return errors.New("invalid volume size")
}
delete(request.Options, "size")

Expand Down
27 changes: 24 additions & 3 deletions storage_drivers/azure/azure_anf.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"context"
"encoding/json"
"fmt"
"math"
"net"
"os"
"reflect"
Expand Down Expand Up @@ -200,10 +201,15 @@ func (d *NASStorageDriver) Initialize(

volumeCreateTimeout := d.defaultCreateTimeout()
if config.VolumeCreateTimeout != "" {
if i, parseErr := strconv.ParseUint(d.Config.VolumeCreateTimeout, 10, 64); parseErr != nil {
i, parseErr := strconv.ParseInt(d.Config.VolumeCreateTimeout, 10, 64)
if parseErr != nil {
Logc(ctx).WithField("interval", d.Config.VolumeCreateTimeout).WithError(parseErr).Error(
"Invalid volume create timeout period.")
return parseErr
} else if i < 0 {
Logc(ctx).WithField("interval", d.Config.VolumeCreateTimeout).WithError(parseErr).Error(
"Unsupported volume create timeout period.")
return errors.UnsupportedError("unsupported volume create timeout period")
} else {
volumeCreateTimeout = time.Duration(i) * time.Second
}
Expand Down Expand Up @@ -559,21 +565,31 @@ func (d *NASStorageDriver) initializeAzureSDKClient(

sdkTimeout := api.DefaultSDKTimeout
if config.SDKTimeout != "" {
if i, parseErr := strconv.ParseUint(d.Config.SDKTimeout, 10, 64); parseErr != nil {
i, parseErr := strconv.ParseInt(d.Config.SDKTimeout, 10, 64)
if parseErr != nil {
Logc(ctx).WithField("interval", d.Config.SDKTimeout).WithError(parseErr).Error(
"Invalid value for SDK timeout.")
return parseErr
} else if i < 0 {
Logc(ctx).WithField("interval", d.Config.SDKTimeout).WithError(parseErr).Error(
"Unsupported value for SDK timeout.")
return errors.UnsupportedError("unsupported SDK timeout")
} else {
sdkTimeout = time.Duration(i) * time.Second
}
}

maxCacheAge := api.DefaultMaxCacheAge
if config.MaxCacheAge != "" {
if i, parseErr := strconv.ParseUint(d.Config.MaxCacheAge, 10, 64); parseErr != nil {
i, parseErr := strconv.ParseInt(d.Config.MaxCacheAge, 10, 64)
if parseErr != nil {
Logc(ctx).WithField("interval", d.Config.MaxCacheAge).WithError(parseErr).Error(
"Invalid value for max cache age.")
return parseErr
} else if i < 0 {
Logc(ctx).WithField("interval", d.Config.MaxCacheAge).WithError(parseErr).Error(
"Unsupported value for max cache age.")
return errors.UnsupportedError("unsupported max cache age")
} else {
maxCacheAge = time.Duration(i) * time.Second
}
Expand Down Expand Up @@ -1912,6 +1928,11 @@ func (d *NASStorageDriver) Resize(ctx context.Context, volConfig *storage.Volume
Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> Resize")
defer Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< Resize")

if sizeBytes > math.MaxInt64 {
Logc(ctx).WithFields(fields).Error("Invalid volume size")
return errors.New("invalid volume size")
}

// Update resource cache as needed
if err := d.SDK.RefreshAzureResources(ctx); err != nil {
return fmt.Errorf("could not update ANF resource cache; %v", err)
Expand Down
27 changes: 24 additions & 3 deletions storage_drivers/azure/azure_anf_subvolume.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"crypto/sha256"
"encoding/json"
"fmt"
"math"
"os"
"reflect"
"regexp"
Expand Down Expand Up @@ -290,10 +291,15 @@ func (d *NASBlockStorageDriver) Initialize(

volumeCreateTimeout := d.defaultCreateTimeout()
if config.VolumeCreateTimeout != "" {
if i, parseErr := strconv.ParseUint(d.Config.VolumeCreateTimeout, 10, 64); parseErr != nil {
i, parseErr := strconv.ParseInt(d.Config.VolumeCreateTimeout, 10, 64)
if parseErr != nil {
Logc(ctx).WithField("interval", d.Config.VolumeCreateTimeout).WithError(parseErr).Error(
"Invalid volume create timeout period.")
return parseErr
} else if i < 0 {
Logc(ctx).WithField("interval", d.Config.VolumeCreateTimeout).WithError(parseErr).Error(
"Unsupported volume create timeout period.")
return errors.UnsupportedError("unsupported volume create timeout period")
} else {
volumeCreateTimeout = time.Duration(i) * time.Second
}
Expand Down Expand Up @@ -548,21 +554,31 @@ func (d *NASBlockStorageDriver) initializeAzureSDKClient(

sdkTimeout := api.DefaultSDKTimeout
if config.SDKTimeout != "" {
if i, parseErr := strconv.ParseUint(d.Config.SDKTimeout, 10, 64); parseErr != nil {
i, parseErr := strconv.ParseInt(d.Config.SDKTimeout, 10, 64)
if parseErr != nil {
Logc(ctx).WithField("interval", d.Config.SDKTimeout).WithError(parseErr).Error(
"Invalid value for SDK timeout.")
return parseErr
} else if i < 0 {
Logc(ctx).WithField("interval", d.Config.SDKTimeout).WithError(parseErr).Error(
"Unsupported value for SDK timeout.")
return errors.UnsupportedError("unsupported SDK timeout")
} else {
sdkTimeout = time.Duration(i) * time.Second
}
}

maxCacheAge := api.DefaultMaxCacheAge
if config.MaxCacheAge != "" {
if i, parseErr := strconv.ParseUint(d.Config.MaxCacheAge, 10, 64); parseErr != nil {
i, parseErr := strconv.ParseInt(d.Config.MaxCacheAge, 10, 64)
if parseErr != nil {
Logc(ctx).WithField("interval", d.Config.MaxCacheAge).WithError(parseErr).Error(
"Invalid value for max cache age.")
return parseErr
} else if i < 0 {
Logc(ctx).WithField("interval", d.Config.MaxCacheAge).WithError(parseErr).Error(
"Unsupported value for max cache age.")
return errors.UnsupportedError("unsupported max cache age")
} else {
maxCacheAge = time.Duration(i) * time.Second
}
Expand Down Expand Up @@ -1686,6 +1702,11 @@ func (d *NASBlockStorageDriver) Resize(ctx context.Context, volConfig *storage.V
Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> Resize")
defer Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< Resize")

if sizeBytes > math.MaxInt64 {
Logc(ctx).WithFields(fields).Error("Invalid volume size")
return errors.New("invalid volume size")
}

// Get the subvolume
subvolumeWithMetadata, err := d.SDK.Subvolume(ctx, volConfig, true)
if err != nil {
Expand Down
31 changes: 31 additions & 0 deletions storage_drivers/azure/azure_anf_subvolume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,37 @@ func TestSubvolumeInitialize_InvalidSDKTimeout(t *testing.T) {
assert.False(t, driver.Initialized(), "initialized")
}

func TestSubvolumeInitialize_NegativeVolumeCreateTimeout(t *testing.T) {
commonConfig, filesystems := getStructsForSubvolumeInitialize()

configJSON := `
{
"version": 1,
"storageDriverName": "azure-netapp-files-subvolume",
"location": "fake-location",
"subscriptionID": "deadbeef-173f-4bf4-b5b8-f17f8d2fe43b",
"tenantID": "deadbeef-4746-4444-a919-3b34af5f0a3c",
"clientID": "deadbeef-784c-4b35-8329-460f52a3ad50",
"clientSecret": "myClientSecret",
"serviceLevel": "Premium",
"debugTraceFlags": {"method": true, "api": true, "discovery": true},
"capacityPools": ["RG1/NA1/CP1", "RG1/NA1/CP2"],
"filePoolVolumes": ["RG1/NA1/CP1/VOL-1"],
"virtualNetwork": "VN1",
"subnet": "RG1/VN1/SN1",
"volumeCreateTimeout": "-600"
}`

mockAPI, driver := newMockANFSubvolumeDriver(t)
mockAPI.EXPECT().ValidateFilePoolVolumes(ctx, gomock.Any()).Return(filesystems, nil).Times(1)
mockAPI.EXPECT().Init(ctx, gomock.Any()).Return(nil).Times(1)
result := driver.Initialize(ctx, tridentconfig.ContextCSI, configJSON, commonConfig, map[string]string{},
BackendUUID)

assert.Error(t, result, "initialized")
assert.False(t, driver.Initialized(), "initialized")
}

func TestSubvolumeInitialize_InvalidVolumeCreateTimeout(t *testing.T) {
commonConfig, filesystems := getStructsForSubvolumeInitialize()

Expand Down
48 changes: 47 additions & 1 deletion storage_drivers/azure/azure_anf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,52 @@ func TestInitialize_InvalidStoragePrefix(t *testing.T) {
assert.False(t, driver.Initialized(), "initialized")
}

func TestInitialize_NegativeVolumeCreateTimeout(t *testing.T) {
commonConfig := &drivers.CommonStorageDriverConfig{
Version: 1,
StorageDriverName: "azure-netapp-files",
BackendName: "myANFBackend",
DriverContext: tridentconfig.ContextCSI,
DebugTraceFlags: debugTraceFlags,
}

configJSON := `
{
"version": 1,
"storageDriverName": "azure-netapp-files",
"location": "fake-location",
"subscriptionID": "deadbeef-173f-4bf4-b5b8-f17f8d2fe43b",
"tenantID": "deadbeef-4746-4444-a919-3b34af5f0a3c",
"clientID": "deadbeef-784c-4b35-8329-460f52a3ad50",
"clientSecret": "myClientSecret",
"serviceLevel": "Premium",
"debugTraceFlags": {"method": true, "api": true, "discovery": true},
"capacityPools": ["RG1/NA1/CP1", "RG1/NA1/CP2"],
"virtualNetwork": "VN1",
"subnet": "RG1/VN1/SN1",
"volumeCreateTimeout": "-600"
}`

// Have to at least one CapacityPool for ANF backends.
pool := &api.CapacityPool{
Name: "CP1",
Location: "fake-location",
NetAppAccount: "NA1",
ResourceGroup: "RG1",
}

mockAPI, driver := newMockANFDriver(t)

mockAPI.EXPECT().Init(ctx, gomock.Any()).Return(nil).Times(1)
mockAPI.EXPECT().CapacityPoolsForStoragePools(ctx).Return([]*api.CapacityPool{pool}).Times(1)

result := driver.Initialize(ctx, tridentconfig.ContextCSI, configJSON, commonConfig, map[string]string{},
BackendUUID)

assert.Error(t, result, "initialize did not fail")
assert.False(t, driver.Initialized(), "initialized")
}

func TestInitialize_InvalidVolumeCreateTimeout(t *testing.T) {
commonConfig := &drivers.CommonStorageDriverConfig{
Version: 1,
Expand All @@ -780,7 +826,7 @@ func TestInitialize_InvalidVolumeCreateTimeout(t *testing.T) {
"capacityPools": ["RG1/NA1/CP1", "RG1/NA1/CP2"],
"virtualNetwork": "VN1",
"subnet": "RG1/VN1/SN1",
"volumeCreateTimeout": "10m"
"volumeCreateTimeout": "600m"
}`

// Have to at least one CapacityPool for ANF backends.
Expand Down
21 changes: 18 additions & 3 deletions storage_drivers/gcp/gcp_cvs.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"context"
"encoding/json"
"fmt"
"math"
"net"
"reflect"
"regexp"
Expand Down Expand Up @@ -312,11 +313,15 @@ func (d *NFSStorageDriver) populateConfigurationDefaults(
// VolumeCreateTimeoutSeconds is the timeout value in seconds.
volumeCreateTimeout := d.defaultCreateTimeout()
if config.VolumeCreateTimeout != "" {
i, err := strconv.ParseUint(d.Config.VolumeCreateTimeout, 10, 64)
i, err := strconv.ParseInt(d.Config.VolumeCreateTimeout, 10, 64)
if err != nil {
Logc(ctx).WithField("interval", d.Config.VolumeCreateTimeout).Errorf(
"Invalid volume create timeout period. %v", err)
Logc(ctx).WithField("interval", d.Config.VolumeCreateTimeout).WithError(err).Error(
"Invalid volume create timeout period.")
return err
} else if i < 0 {
Logc(ctx).WithField("interval", d.Config.VolumeCreateTimeout).WithError(err).Error(
"Unsupported volume create timeout period.")
return errors.UnsupportedError("unsupported volume create timeout period")
}
volumeCreateTimeout = time.Duration(i) * time.Second
}
Expand Down Expand Up @@ -728,6 +733,11 @@ func (d *NFSStorageDriver) Create(
sizeBytes = d.applyMinimumVolumeSizeHW(requestedSizeBytes)
}

if sizeBytes > math.MaxInt64 {
Logc(ctx).WithFields(fields).Error("Invalid volume size")
return errors.New("invalid volume size")
}

if requestedSizeBytes < sizeBytes {
Logc(ctx).WithFields(LogFields{
"name": name,
Expand Down Expand Up @@ -1770,6 +1780,11 @@ func (d *NFSStorageDriver) Resize(ctx context.Context, volConfig *storage.Volume
Logd(ctx, d.Config.StorageDriverName, d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> Resize")
defer Logd(ctx, d.Config.StorageDriverName, d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< Resize")

if sizeBytes > math.MaxInt64 {
Logc(ctx).WithFields(fields).Error("Invalid volume size")
return errors.New("invalid volume size")
}

// Get the volume
creationToken := name

Expand Down
2 changes: 1 addition & 1 deletion storage_drivers/gcp/gcp_cvs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ func TestInitialize_InvalidVolumeCreateTimeout(t *testing.T) {
d := newTestGCPDriver(nil)

err := d.Initialize(ctx(), tridentconfig.ContextCSI, configJSON, commonConfig, map[string]string{}, "abcd")
assert.ErrorContains(t, err, "strconv.ParseUint", "Valid volume create timeout")
assert.Error(t, err, "Expected an error")
assert.False(t, d.initialized, "Driver initialized")
}

Expand Down
6 changes: 6 additions & 0 deletions storage_drivers/ontap/api/abstraction_rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"encoding/json"
"fmt"
"math"
"runtime/debug"
"sort"
"strconv"
Expand Down Expand Up @@ -1942,6 +1943,11 @@ func (d OntapAPIREST) LunCreate(ctx context.Context, lun Lun) error {

sizeBytesStr, _ := utils.ConvertSizeToBytes(lun.Size)
sizeBytes, _ := strconv.ParseUint(sizeBytesStr, 10, 64)
if sizeBytes > math.MaxInt64 {
Logc(ctx).WithField("sizeInBytes", sizeBytes).Error("Invalid volume size")
return errors.New("invalid volume size")
}

creationErr := d.api.LunCreate(ctx, lun.Name, int64(sizeBytes), lun.OsType, lun.Qos, *lun.SpaceReserved,
*lun.SpaceAllocated)
if creationErr != nil {
Expand Down
14 changes: 14 additions & 0 deletions storage_drivers/ontap/api/ontap_rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"encoding/json"
"fmt"
"io"
"math"
"net/http"
"net/url"
"os"
Expand Down Expand Up @@ -611,6 +612,10 @@ func (c RestClient) setVolumeSizeByNameAndStyle(ctx context.Context, volumeName,

sizeBytesStr, _ := utils.ConvertSizeToBytes(newSize)
sizeBytes, _ := strconv.ParseUint(sizeBytesStr, 10, 64)
if sizeBytes > math.MaxInt64 {
Logc(ctx).WithField("sizeInBytes", sizeBytes).Error("Invalid volume size")
return errors.New("invalid volume size")
}

volumeInfo := &models.Volume{
Size: utils.Ptr(int64(sizeBytes)),
Expand Down Expand Up @@ -2884,6 +2889,11 @@ func (c RestClient) LunSetSize(

sizeBytesStr, _ := utils.ConvertSizeToBytes(newSize)
sizeBytes, _ := strconv.ParseUint(sizeBytesStr, 10, 64)
if sizeBytes > math.MaxInt64 {
Logc(ctx).WithField("sizeInBytes", sizeBytes).Error("Invalid volume size")
return 0, errors.New("invalid volume size")
}

spaceInfo := &models.LunInlineSpace{
Size: utils.Ptr(int64(sizeBytes)),
}
Expand Down Expand Up @@ -5647,6 +5657,10 @@ func (c RestClient) NVMeNamespaceCreate(ctx context.Context, ns NVMeNamespace) (

sizeBytesStr, _ := utils.ConvertSizeToBytes(ns.Size)
sizeInBytes, _ := strconv.ParseUint(sizeBytesStr, 10, 64)
if sizeInBytes > math.MaxInt64 {
Logc(ctx).WithField("sizeInBytes", sizeInBytes).Error("Invalid volume size")
return "", errors.New("invalid volume size")
}

nsInfo := &models.NvmeNamespace{
Name: &ns.Name,
Expand Down
Loading

0 comments on commit 8059fde

Please sign in to comment.