diff --git a/storeapi/go-wrapper/microsoftstore/errors.go b/storeapi/go-wrapper/microsoftstore/errors.go index c48d5a29b..287ad07f6 100644 --- a/storeapi/go-wrapper/microsoftstore/errors.go +++ b/storeapi/go-wrapper/microsoftstore/errors.go @@ -2,7 +2,7 @@ package microsoftstore // StoreAPIError are the error constants in the store api. -type StoreAPIError int +type StoreAPIError int64 // Keep up-to-date with `storeapi\base\Exception.hpp`. const ( @@ -26,7 +26,7 @@ const ( ) // NewStoreAPIError creates StoreAPIError from the result of a call to the storeAPI DLL. -func NewStoreAPIError(hresult uintptr) error { +func NewStoreAPIError(hresult int64) error { if err := StoreAPIError(hresult); err < ErrSuccess { return err } diff --git a/storeapi/go-wrapper/microsoftstore/export_test.go b/storeapi/go-wrapper/microsoftstore/export_test.go index d893d5df4..ee7467f26 100644 --- a/storeapi/go-wrapper/microsoftstore/export_test.go +++ b/storeapi/go-wrapper/microsoftstore/export_test.go @@ -2,3 +2,6 @@ package microsoftstore // FindWorkspaceRoot climbs up the current working directory until the Go workspace root is found. var FindWorkspaceRoot = findWorkspaceRoot + +// CheckError inspects the values of hres and err to determine what kind of error we have, if any, according to the rules of syscall/dll_windows.go. +var CheckError = checkError diff --git a/storeapi/go-wrapper/microsoftstore/store.go b/storeapi/go-wrapper/microsoftstore/store.go index ef792e710..0f0c1db33 100644 --- a/storeapi/go-wrapper/microsoftstore/store.go +++ b/storeapi/go-wrapper/microsoftstore/store.go @@ -2,8 +2,10 @@ package microsoftstore import ( "errors" + "fmt" "os" "path/filepath" + "syscall" ) // findWorkspaceRoot climbs up the current working directory until the Go workspace root is found. @@ -26,3 +28,40 @@ func findWorkspaceRoot() (string, error) { } } } + +// checkError inspects the values of hres and err to determine what kind of error we have, if any, according to the rules of syscall/dll_windows.go. +func checkError(hres int64, err error) (int64, error) { + // From syscall/dll_windows.go (*Proc).Call doc: + // > Callers must inspect the primary return value to decide whether an + // error occurred [...] before consulting the error. + // There is no possibility of nil error, the `err` return value is always constructed with the + // result of `GetLastError()` which could have been set by something completely + // unrelated to our code some time in the past, as well as it could be `ERROR_SUCCESS` which is the `Errno(0)`. + // If the act of calling the API fails (not the function we're calling, but the attempt to call it), then we'd + // have a meaningful `syscall.Errno` object via the `err` parameter, related to the actual failure (like a function not found in this DLL) + // Since our implementation of the store API doesn't touch errno the call should return `hres` + // in our predefined range plus garbage in the `err` argument, thus we only care about the `hres` in this case. + if e := NewStoreAPIError(hres); e != nil { + return hres, fmt.Errorf("storeApi returned error code %d: %w", hres, e) + } + + // Supposedly unreachable: proc.Call must always return a non-nil syscall.Errno + if err == nil { + return hres, nil + } + + var target syscall.Errno + if b := errors.As(err, &target); !b { + // Supposedly unreachable: proc.Call must always return a non-nil syscall.Errno + return hres, err + } + + // The act of calling our API didn't succeed, function not found in the DLL for example: + if target != syscall.Errno(0) { + return hres, fmt.Errorf("failed syscall to storeApi: %v (syscall errno %d)", target, err) + } + + // A non-error value in hres plus ERROR_SUCCESS in err. + // This shouldn't happen in the current store API implementation anyway. + return hres, nil +} diff --git a/storeapi/go-wrapper/microsoftstore/store_test.go b/storeapi/go-wrapper/microsoftstore/store_test.go index b502a797b..12f862de4 100644 --- a/storeapi/go-wrapper/microsoftstore/store_test.go +++ b/storeapi/go-wrapper/microsoftstore/store_test.go @@ -2,12 +2,14 @@ package microsoftstore_test import ( "context" + "errors" "fmt" "log/slog" "os" "os/exec" "path/filepath" "runtime" + "syscall" "testing" "time" @@ -87,6 +89,39 @@ func TestGetSubscriptionExpirationDate(t *testing.T) { require.ErrorIs(t, gotErr, wantErr, "GetSubscriptionExpirationDate should have returned code %d", wantErr) } +func TestErrorVerification(t *testing.T) { + t.Parallel() + testcases := map[string]struct { + hresult int64 + err error + + wantErr bool + }{ + "Success": {}, + // If HRESULT is not in the Store API error range and err is not a syscall.Errno then we don't have an error. + "With an unknown value (not an error)": {hresult: 1, wantErr: false}, + + "Upper bound of the Store API enum range": {hresult: -1, wantErr: true}, + "Lower bound of the Store API enum range": {hresult: int64(microsoftstore.ErrNotSubscribed), wantErr: true}, + "With a system error (errno)": {hresult: 32 /*garbage*/, err: syscall.Errno(2) /*E_FILE_NOT_FOUND*/, wantErr: true}, + "With a generic (unreachable) error": {hresult: 1, err: errors.New("test error"), wantErr: true}, + // This would mean an API call returning a non-error hresult plus GetLastError() returning ERROR_SUCCESS + // This shouldn't happen in the current store API implementation anyway. + "With weird successful error": {hresult: 1, err: syscall.Errno(0) /*ERROR_SUCCESS*/}, + } + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + t.Parallel() + res, err := microsoftstore.CheckError(tc.hresult, tc.err) + if tc.wantErr { + require.Error(t, err, "CheckError should have returned an error for value: %v, returned value was: %v", tc.hresult, res) + return + } + require.NoError(t, err, "CheckError should have not returned an error for value: %v, returned value was: %v", tc.hresult, res) + }) + } +} + func buildStoreAPI(ctx context.Context) error { ctx, cancel := context.WithTimeout(ctx, 5*time.Minute) defer cancel() diff --git a/storeapi/go-wrapper/microsoftstore/store_windows.go b/storeapi/go-wrapper/microsoftstore/store_windows.go index d9bf10754..2a79967d2 100644 --- a/storeapi/go-wrapper/microsoftstore/store_windows.go +++ b/storeapi/go-wrapper/microsoftstore/store_windows.go @@ -2,7 +2,6 @@ package microsoftstore import ( - "errors" "fmt" "sync" "syscall" @@ -81,7 +80,7 @@ func GetSubscriptionExpirationDate() (tm time.Time, err error) { // Use this instead of proc.Call to avoid panics. // //nolint:unparam // Return value is provided to follow convention. -func call(proc *syscall.LazyProc, args ...uintptr) (int, error) { +func call(proc *syscall.LazyProc, args ...uintptr) (int64, error) { if err := loadDll(); err != nil { return 0, err } @@ -92,29 +91,8 @@ func call(proc *syscall.LazyProc, args ...uintptr) (int, error) { } hresult, _, err := proc.Call(args...) - - // From syscall/dll_windows.go (*Proc).Call doc: - // > Callers must inspect the primary return value to decide whether an - // error occurred [...] before consulting the error. - if err := NewStoreAPIError(hresult); err != nil { - return int(hresult), fmt.Errorf("storeApi returned error code %d: %w", int(hresult), err) - } - - if err == nil { - return int(hresult), nil - } - - var target syscall.Errno - if b := errors.As(err, &target); !b { - // Supposedly unrechable: proc.Call must always return a syscall.Errno - return int(hresult), err - } - - if target != syscall.Errno(0) { - return int(hresult), fmt.Errorf("failed syscall to storeApi: %v (syscall errno %d)", target, err) - } - - return int(hresult), nil + //nolint:gosec // Windows HRESULTS are guaranteed to be 32-bit vlaue, thus they surely fit inside a int64 without overflow. + return checkError(int64(hresult), err) } // loadDll finds the dll and ensures it loads. diff --git a/tools/go.mod b/tools/go.mod index 979c91c98..064b565cf 100644 --- a/tools/go.mod +++ b/tools/go.mod @@ -6,7 +6,7 @@ toolchain go1.22.5 require ( github.com/canonical/ubuntu-pro-for-wsl/windows-agent v0.0.0-20240626165418-426bf0d3695a github.com/canonical/ubuntu-pro-for-wsl/wsl-pro-service v0.0.0-20240705150615-15d23a639880 - github.com/golangci/golangci-lint v1.60.1 + github.com/golangci/golangci-lint v1.60.2 github.com/spf13/cobra v1.8.1 github.com/stretchr/testify v1.9.0 golang.org/x/vuln v1.1.3 @@ -85,7 +85,7 @@ require ( github.com/gofrs/flock v0.12.1 // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a // indirect - github.com/golangci/gofmt v0.0.0-20231018234816-f50ced29576e // indirect + github.com/golangci/gofmt v0.0.0-20240816233607-d8596aa466a9 // indirect github.com/golangci/misspell v0.6.0 // indirect github.com/golangci/modinfo v0.3.4 // indirect github.com/golangci/plugin-module-register v0.1.1 // indirect @@ -156,7 +156,7 @@ require ( github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 // indirect github.com/sashamelentyev/interfacebloat v1.1.0 // indirect github.com/sashamelentyev/usestdlibvars v1.27.0 // indirect - github.com/securego/gosec/v2 v2.20.1-0.20240525090044-5f0084eb01a9 // indirect + github.com/securego/gosec/v2 v2.20.1-0.20240820084340-81cda2f91fbe // indirect github.com/shazow/go-diff v0.0.0-20160112020656-b6b7b6733b8c // indirect github.com/sirupsen/logrus v1.9.3 // indirect github.com/sivchari/containedctx v1.0.3 // indirect @@ -177,7 +177,7 @@ require ( github.com/tetafro/godot v1.4.16 // indirect github.com/timakin/bodyclose v0.0.0-20230421092635-574207250966 // indirect github.com/timonwong/loggercheck v0.9.4 // indirect - github.com/tomarrell/wrapcheck/v2 v2.8.3 // indirect + github.com/tomarrell/wrapcheck/v2 v2.9.0 // indirect github.com/tommy-muehle/go-mnd/v2 v2.5.1 // indirect github.com/ubuntu/decorate v0.0.0-20240425133904-a085253511fb // indirect github.com/ubuntu/gowsl v0.0.0-20240709100851-bc766d3dbd53 // indirect @@ -195,7 +195,7 @@ require ( go.uber.org/automaxprocs v1.5.3 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.24.0 // indirect - golang.org/x/exp v0.0.0-20240707233637-46b078467d37 // indirect + golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect golang.org/x/exp/typeparams v0.0.0-20240314144324-c7f7c6466f7f // indirect golang.org/x/mod v0.20.0 // indirect golang.org/x/net v0.28.0 // indirect @@ -204,11 +204,11 @@ require ( golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7 // indirect golang.org/x/text v0.17.0 // indirect golang.org/x/tools v0.24.0 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20240709173604-40e1e62336c5 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20240730163845-b1a4ccb954bf // indirect google.golang.org/grpc v1.65.0 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - honnef.co/go/tools v0.5.0 // indirect - mvdan.cc/gofumpt v0.6.0 // indirect + honnef.co/go/tools v0.5.1 // indirect + mvdan.cc/gofumpt v0.7.0 // indirect mvdan.cc/unparam v0.0.0-20240528143540-8a5130ca722f // indirect ) diff --git a/tools/go.sum b/tools/go.sum index b3a4b3b5e..9e73b3200 100644 --- a/tools/go.sum +++ b/tools/go.sum @@ -183,8 +183,10 @@ github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vb github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A= -github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= -github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= +github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-quicktest/qt v1.101.0 h1:O1K29Txy5P2OK0dGo59b7b0LR6wKfIhttaAhHUyn7eI= +github.com/go-quicktest/qt v1.101.0/go.mod h1:14Bz/f7NwaXPtdYEgzsx46kqSxVwTbzVZsDC26tQJow= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= @@ -247,10 +249,10 @@ github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a h1:w8hkcTqaFpzKqonE9uMCefW1WDie15eSP/4MssdenaM= github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a/go.mod h1:ryS0uhF+x9jgbj/N71xsEqODy9BN81/GonCZiOzirOk= -github.com/golangci/gofmt v0.0.0-20231018234816-f50ced29576e h1:ULcKCDV1LOZPFxGZaA6TlQbiM3J2GCPnkx/bGF6sX/g= -github.com/golangci/gofmt v0.0.0-20231018234816-f50ced29576e/go.mod h1:Pm5KhLPA8gSnQwrQ6ukebRcapGb/BG9iUkdaiCcGHJM= -github.com/golangci/golangci-lint v1.60.1 h1:DRKNqNTQRLBJZ1il5u4fvgLQCjQc7QFs0DbhksJtVJE= -github.com/golangci/golangci-lint v1.60.1/go.mod h1:jDIPN1rYaIA+ijp9OZcUmUCoQOtZ76pOlFbi15FlLJY= +github.com/golangci/gofmt v0.0.0-20240816233607-d8596aa466a9 h1:/1322Qns6BtQxUZDTAT4SdcoxknUki7IAoK4SAXr8ME= +github.com/golangci/gofmt v0.0.0-20240816233607-d8596aa466a9/go.mod h1:Oesb/0uFAyWoaw1U1qS5zyjCg5NP9C9iwjnI4tIsXEE= +github.com/golangci/golangci-lint v1.60.2 h1:Y8aWnZCMOLY5T7Ga5hcoemyKsZZJCUmIIK3xTD3jIhc= +github.com/golangci/golangci-lint v1.60.2/go.mod h1:4UvjLpOJoQSvmyWkmO1urDR3txhL9R9sn4oM/evJ95g= github.com/golangci/misspell v0.6.0 h1:JCle2HUTNWirNlDIAUO44hUsKhOFqGPoC4LZxlaSXDs= github.com/golangci/misspell v0.6.0/go.mod h1:keMNyY6R9isGaSAu+4Q8NMBwMPkh15Gtc8UCVoDtAWo= github.com/golangci/modinfo v0.3.4 h1:oU5huX3fbxqQXdfspamej74DFX0kyGLkw1ppvXoJ8GA= @@ -289,8 +291,8 @@ github.com/google/pprof v0.0.0-20200212024743-f11f1df84d12/go.mod h1:ZgVRPoUq/hf github.com/google/pprof v0.0.0-20200229191704-1ebb73c60ed3/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM= github.com/google/pprof v0.0.0-20200430221834-fc25d7d30c6d/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM= github.com/google/pprof v0.0.0-20200708004538-1a94d8640e99/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM= -github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6 h1:k7nVchz72niMH6YLQNvHSdIE7iqsQxK1P41mySCvssg= -github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6/go.mod h1:kf6iHlnVGwgKolg33glAes7Yg/8iWP8ukqeldJSO7jw= +github.com/google/pprof v0.0.0-20240727154555-813a5fbdbec8 h1:FKHo8hFI3A+7w0aUQuYXQ+6EN5stWmeY/AZqtM8xk9k= +github.com/google/pprof v0.0.0-20240727154555-813a5fbdbec8/go.mod h1:K1liHPHnj73Fdn/EKuT8nrFqBihUSKXoLYU0BuatOYo= github.com/google/renameio v0.1.0 h1:GOZbcHa3HfsPKPlmyPyN2KEohoMXOhdMbHrvbpl2QaA= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= @@ -421,10 +423,10 @@ github.com/nunnatsa/ginkgolinter v0.16.2 h1:8iLqHIZvN4fTLDC0Ke9tbSZVcyVHoBs0HIbn github.com/nunnatsa/ginkgolinter v0.16.2/go.mod h1:4tWRinDN1FeJgU+iJANW/kz7xKN5nYRAOfJDQUS9dOQ= github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec= github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY= -github.com/onsi/ginkgo/v2 v2.17.3 h1:oJcvKpIb7/8uLpDDtnQuf18xVnwKp8DTD7DQ6gTd/MU= -github.com/onsi/ginkgo/v2 v2.17.3/go.mod h1:nP2DPOQoNsQmsVyv5rDA8JkXQoCs6goXIvr/PRJ1eCc= -github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk= -github.com/onsi/gomega v1.33.1/go.mod h1:U4R44UsT+9eLIaYRB2a5qajjtQYn0hauxvRm16AVYg0= +github.com/onsi/ginkgo/v2 v2.20.0 h1:PE84V2mHqoT1sglvHc8ZdQtPcwmvvt29WLEEO3xmdZw= +github.com/onsi/ginkgo/v2 v2.20.0/go.mod h1:lG9ey2Z29hR41WMVthyJBGUBcBhGOtoPF2VFMvBXFCI= +github.com/onsi/gomega v1.34.1 h1:EUMJIKUjM8sKjYbtxQI9A4z2o+rruxnzNvpknOXie6k= +github.com/onsi/gomega v1.34.1/go.mod h1:kU1QgUvBDLXBJq618Xvm2LUX6rSAfRaFRTcdOeDLwwY= github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw= github.com/otiai10/copy v1.14.0 h1:dCI/t1iTdYGtkvCuBG2BgR6KZa83PTclw4U5n2wAllU= github.com/otiai10/copy v1.14.0/go.mod h1:ECfuL02W+/FkTWZWgQqXPWZgW9oeKCSQ5qVfSc4qc4w= @@ -499,8 +501,8 @@ github.com/sashamelentyev/interfacebloat v1.1.0 h1:xdRdJp0irL086OyW1H/RTZTr1h/tM github.com/sashamelentyev/interfacebloat v1.1.0/go.mod h1:+Y9yU5YdTkrNvoX0xHc84dxiN1iBi9+G8zZIhPVoNjQ= github.com/sashamelentyev/usestdlibvars v1.27.0 h1:t/3jZpSXtRPRf2xr0m63i32ZrusyurIGT9E5wAvXQnI= github.com/sashamelentyev/usestdlibvars v1.27.0/go.mod h1:9nl0jgOfHKWNFS43Ojw0i7aRoS4j6EBye3YBhmAIRF8= -github.com/securego/gosec/v2 v2.20.1-0.20240525090044-5f0084eb01a9 h1:rnO6Zp1YMQwv8AyxzuwsVohljJgp4L0ZqiCgtACsPsc= -github.com/securego/gosec/v2 v2.20.1-0.20240525090044-5f0084eb01a9/go.mod h1:dg7lPlu/xK/Ut9SedURCoZbVCR4yC7fM65DtH9/CDHs= +github.com/securego/gosec/v2 v2.20.1-0.20240820084340-81cda2f91fbe h1:exdneYmXwZ4+VaIWv9mQ47uIHkTQSN50DYdCjXJ1cdQ= +github.com/securego/gosec/v2 v2.20.1-0.20240820084340-81cda2f91fbe/go.mod h1:iyeMMRw8QEmueUSZ2VqmkQMiDyDcobfPnG00CV/NWdE= github.com/shazow/go-diff v0.0.0-20160112020656-b6b7b6733b8c h1:W65qqJCIOVP4jpqPQ0YvHYKwcMEMVWIzWC5iNQQfBTU= github.com/shazow/go-diff v0.0.0-20160112020656-b6b7b6733b8c/go.mod h1:/PevMnwAxekIXwN8qQyfc5gl2NlkB3CQlkizAbOkeBs= github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e/go.mod h1:TDJrrUr11Vxrven61rcy3hJMUqaf/CLWYhHNPmT14Lk= @@ -566,8 +568,8 @@ github.com/timakin/bodyclose v0.0.0-20230421092635-574207250966 h1:quvGphlmUVU+n github.com/timakin/bodyclose v0.0.0-20230421092635-574207250966/go.mod h1:27bSVNWSBOHm+qRp1T9qzaIpsWEP6TbUnei/43HK+PQ= github.com/timonwong/loggercheck v0.9.4 h1:HKKhqrjcVj8sxL7K77beXh0adEm6DLjV/QOGeMXEVi4= github.com/timonwong/loggercheck v0.9.4/go.mod h1:caz4zlPcgvpEkXgVnAJGowHAMW2NwHaNlpS8xDbVhTg= -github.com/tomarrell/wrapcheck/v2 v2.8.3 h1:5ov+Cbhlgi7s/a42BprYoxsr73CbdMUTzE3bRDFASUs= -github.com/tomarrell/wrapcheck/v2 v2.8.3/go.mod h1:g9vNIyhb5/9TQgumxQyOEqDHsmGYcGsVMOx/xGkqdMo= +github.com/tomarrell/wrapcheck/v2 v2.9.0 h1:801U2YCAjLhdN8zhZ/7tdjB3EnAoRlJHt/s+9hijLQ4= +github.com/tomarrell/wrapcheck/v2 v2.9.0/go.mod h1:g9vNIyhb5/9TQgumxQyOEqDHsmGYcGsVMOx/xGkqdMo= github.com/tommy-muehle/go-mnd/v2 v2.5.1 h1:NowYhSdyE/1zwK9QCLeRb6USWdoif80Ie+v+yU8u1Zw= github.com/tommy-muehle/go-mnd/v2 v2.5.1/go.mod h1:WsUAkMJMYww6l/ufffCD3m+P7LEvr8TnZn9lwVDlgzw= github.com/ubuntu/decorate v0.0.0-20240425133904-a085253511fb h1:8aYaLeK5kwneVflZ50XOnsE2zWnS3HCF58WV8JXge5U= @@ -636,8 +638,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= -golang.org/x/exp v0.0.0-20240707233637-46b078467d37 h1:uLDX+AfeFCct3a2C7uIWBKMJIR3CJMhcgfrUAqjRK6w= -golang.org/x/exp v0.0.0-20240707233637-46b078467d37/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY= +golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8= +golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY= golang.org/x/exp/typeparams v0.0.0-20220428152302-39d4317da171/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= golang.org/x/exp/typeparams v0.0.0-20230203172020-98cc5a0785f9/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= golang.org/x/exp/typeparams v0.0.0-20240314144324-c7f7c6466f7f h1:phY1HzDcf18Aq9A8KkmRtY9WvOFIxN8wgfvy6Zm1DV8= @@ -929,8 +931,8 @@ google.golang.org/genproto v0.0.0-20200618031413-b414f8b61790/go.mod h1:jDfRM7Fc google.golang.org/genproto v0.0.0-20200729003335-053ba62fc06f/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20200804131852-c06518451d9c/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20200825200019-8632dd797987/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240709173604-40e1e62336c5 h1:SbSDUWW1PAO24TNpLdeheoYPd7kllICcLU52x6eD4kQ= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240709173604-40e1e62336c5/go.mod h1:Ue6ibwXGpU+dqIcODieyLOcgj7z8+IcskoNIgZxtrFY= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240730163845-b1a4ccb954bf h1:liao9UHurZLtiEwBgT9LMOnKYsHze6eA6w1KQCMVN2Q= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240730163845-b1a4ccb954bf/go.mod h1:Ue6ibwXGpU+dqIcODieyLOcgj7z8+IcskoNIgZxtrFY= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= @@ -987,10 +989,10 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= -honnef.co/go/tools v0.5.0 h1:29uoiIormS3Z6R+t56STz/oI4v+mB51TSmEOdJPgRnE= -honnef.co/go/tools v0.5.0/go.mod h1:e9irvo83WDG9/irijV44wr3tbhcFeRnfpVlRqVwpzMs= -mvdan.cc/gofumpt v0.6.0 h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo= -mvdan.cc/gofumpt v0.6.0/go.mod h1:4L0wf+kgIPZtcCWXynNS2e6bhmj73umwnuXSZarixzA= +honnef.co/go/tools v0.5.1 h1:4bH5o3b5ZULQ4UrBmP+63W9r7qIkqJClEA9ko5YKx+I= +honnef.co/go/tools v0.5.1/go.mod h1:e9irvo83WDG9/irijV44wr3tbhcFeRnfpVlRqVwpzMs= +mvdan.cc/gofumpt v0.7.0 h1:bg91ttqXmi9y2xawvkuMXyvAA/1ZGJqYAEGjXuP0JXU= +mvdan.cc/gofumpt v0.7.0/go.mod h1:txVFJy/Sc/mvaycET54pV8SW8gWxTlUuGHVEcncmNUo= mvdan.cc/unparam v0.0.0-20240528143540-8a5130ca722f h1:lMpcwN6GxNbWtbpI1+xzFLSW8XzX0u72NttUGVFjO3U= mvdan.cc/unparam v0.0.0-20240528143540-8a5130ca722f/go.mod h1:RSLa7mKKCNeTTMHBw5Hsy2rfJmd6O2ivt9Dw9ZqCQpQ= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= diff --git a/windows-agent/internal/daemon/daemon_test.go b/windows-agent/internal/daemon/daemon_test.go index 08fe629f7..2fd4e1ff4 100644 --- a/windows-agent/internal/daemon/daemon_test.go +++ b/windows-agent/internal/daemon/daemon_test.go @@ -169,10 +169,11 @@ func TestServeWSLIP(t *testing.T) { "With mirrored networking mode": {netmode: "mirrored", withAdapters: daemontestutils.MultipleHyperVAdaptersInList}, "With no access to the system distro but net mode is the default (NAT)": {netmode: "error", withAdapters: daemontestutils.MultipleHyperVAdaptersInList}, - "Error when the networking mode is unknown": {netmode: "unknown", wantErr: true}, - "Error when the list of adapters is empty": {withAdapters: daemontestutils.EmptyList, wantErr: true}, - "Error when there is no Hyper-V adapter the list": {withAdapters: daemontestutils.NoHyperVAdapterInList, wantErr: true}, - "Error when retrieving adapters information fails": {withAdapters: daemontestutils.MockError, wantErr: true}, + "Error when the networking mode is unknown": {netmode: "unknown", wantErr: true}, + "Error when the list of adapters is empty": {withAdapters: daemontestutils.EmptyList, wantErr: true}, + "Error when listing adapters requires too much memory": {withAdapters: daemontestutils.RequiresTooMuchMem, wantErr: true}, + "Error when there is no Hyper-V adapter the list": {withAdapters: daemontestutils.NoHyperVAdapterInList, wantErr: true}, + "Error when retrieving adapters information fails": {withAdapters: daemontestutils.MockError, wantErr: true}, } for name, tc := range testcases { diff --git a/windows-agent/internal/daemon/daemontestutils/networking_mock.go b/windows-agent/internal/daemon/daemontestutils/networking_mock.go index 1360c023c..9d0bd9721 100644 --- a/windows-agent/internal/daemon/daemontestutils/networking_mock.go +++ b/windows-agent/internal/daemon/daemontestutils/networking_mock.go @@ -2,6 +2,7 @@ package daemontestutils import ( "errors" + "math" "net" "unsafe" @@ -15,6 +16,9 @@ const ( // MockError is a state that causes the GetAdaptersAddresses to always return an error. MockError MockIPAdaptersState = iota + // RequiresTooMuchMem is a state that causes the GetAdaptersAddresses to request allocation of MaxUint32 (over the capacity of the real Win32 API). + RequiresTooMuchMem + // EmptyList is a state that causes the GetAdaptersAddresses to return an empty list of adapters. EmptyList @@ -96,6 +100,9 @@ func (m *MockIPConfig) GetAdaptersAddresses(_, _ uint32, _ uintptr, adapterAddre switch m.state { case MockError: return errors.New("mock error") + case RequiresTooMuchMem: + *sizePointer = math.MaxUint32 + return ERROR_BUFFER_OVERFLOW case EmptyList: return nil default: @@ -105,24 +112,29 @@ func (m *MockIPConfig) GetAdaptersAddresses(_, _ uint32, _ uintptr, adapterAddre // fillBufferFromTemplate fills a pre-allocated buffer of ipAdapterAddresses with the data from the mockIPAddrsTemplate. func fillBufferFromTemplate(adaptersAddresses *IPAdapterAddresses, sizePointer *uint32, mockIPAddrsTemplate []MockIPAddrsTemplate) error { - count := uint32(len(mockIPAddrsTemplate)) - objSize := uint32(unsafe.Sizeof(IPAdapterAddresses{})) + count := len(mockIPAddrsTemplate) + objSize := int(unsafe.Sizeof(IPAdapterAddresses{})) bufSizeNeeded := count * objSize - if *sizePointer < bufSizeNeeded { + if bufSizeNeeded >= math.MaxUint32 || bufSizeNeeded < 0 { + return errors.New("buffer size limit reached") + } + //nolint:gosec // Value guaranteed to fit inside uint32. + bufSz := uint32(bufSizeNeeded) + if *sizePointer < bufSz { return ERROR_BUFFER_OVERFLOW } //nolint:gosec // Using unsafe to manipulate pointers mimicking the Win32 API, only used in tests. begin := unsafe.Pointer(adaptersAddresses) for _, addr := range mockIPAddrsTemplate { - next := unsafe.Add(begin, int(objSize)) // next = ++begin + next := unsafe.Add(begin, objSize) // next = ++begin ptr := (*IPAdapterAddresses)(begin) fillFromTemplate(&addr, ptr, (*IPAdapterAddresses)(next)) begin = next } - *sizePointer = bufSizeNeeded + *sizePointer = bufSz return nil } diff --git a/windows-agent/internal/daemon/networking.go b/windows-agent/internal/daemon/networking.go index a7c2bd37e..ed916caac 100644 --- a/windows-agent/internal/daemon/networking.go +++ b/windows-agent/internal/daemon/networking.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "math" "net" "os/exec" "reflect" @@ -139,15 +140,20 @@ func getAddrList(opts options) (head *ipAdapterAddresses, err error) { // the buffer around while we're using it, invalidating the NEXT pointers. var buff buffer[ipAdapterAddresses] - // Win32 API docs recommend a buff size of 15KB. - buff.resizeBytes(15 * kilobyte) + // Win32 API docs recommend a buff size of 15KB to start. + size, err := buff.resizeBytes(15 * kilobyte) + // This error condition should be impossible. + if err != nil { + return nil, err + } for range 10 { - size := buff.byteCount() - err := opts.getAdaptersAddresses(family, flags, 0, &buff.data[0], &size) + err = opts.getAdaptersAddresses(family, flags, 0, &buff.data[0], &size) if errors.Is(err, ERROR_BUFFER_OVERFLOW) { // Buffer too small, try again with the returned size. - buff.resizeBytes(size) + if size, err = buff.resizeBytes(size); err != nil { + return nil, err + } continue } if err != nil { @@ -171,27 +177,31 @@ type buffer[T any] struct { data []T } -// byteCount returns the number of bytes in the buffer. -func (b buffer[T]) byteCount() uint32 { - var t T - sizeOf := uint32(reflect.TypeOf(t).Size()) - n := uint32(len(b.data)) - return n * sizeOf -} - // ResizeBytes resizes the buffer to the given number of bytes, rounded UP to fit an integer element size. -func (b *buffer[T]) resizeBytes(n uint32) { +func (b *buffer[T]) resizeBytes(n uint32) (uint32, error) { var t T - sizeOf := uint32(reflect.TypeOf(t).Size()) + n64 := uint64(n) + sizeOf := uint64(reflect.TypeOf(t).Size()) - newLen := int(n / sizeOf) - if n%sizeOf != 0 { + newLen := n64 / sizeOf + if n64%sizeOf != 0 { newLen++ } - if newLen > len(b.data) { + // the sizes the Win32 API GetAdaptersAddresses works with are uint32, thus we cannot allocate + // more than MaxUint32 bytes after all. + newSize := newLen * sizeOf + if newSize >= math.MaxUint32 { + return 0, errors.New("buffer allocated size limit reached") + } + + if newLen > uint64(len(b.data)) { b.data = make([]T, newLen) + // Since make() guarantees len(b.data) == newLen, there is no need to recompute it. } + + //nolint:gosec //uint64 -> uint32 conversion is safe because we checked that newSize < MaxUint32. + return uint32(newSize), nil } // ptr returns a pointer to the start of the buffer. diff --git a/windows-agent/internal/proservices/landscape/distroinstall/distroinstall.go b/windows-agent/internal/proservices/landscape/distroinstall/distroinstall.go index 075c325f5..ef5ed8402 100644 --- a/windows-agent/internal/proservices/landscape/distroinstall/distroinstall.go +++ b/windows-agent/internal/proservices/landscape/distroinstall/distroinstall.go @@ -69,6 +69,7 @@ func CreateUser(ctx context.Context, d gowsl.Distro, userName string, userFullNa return 0, fmt.Errorf("could not parse uid %q: %v", string(out), err) } + //nolint:gosec // strconv.ParseUint with bitSize 32 ensures the value of id64 fits inside uint32. return uint32(id64), nil } diff --git a/wsl-pro-service/internal/system/networking.go b/wsl-pro-service/internal/system/networking.go index d0c09ab11..1d27b516d 100644 --- a/wsl-pro-service/internal/system/networking.go +++ b/wsl-pro-service/internal/system/networking.go @@ -101,6 +101,7 @@ func (s *System) defaultGateway() (ip net.IP, err error) { } b := make([]byte, 4) + //nolint:gosec // Value is guaranteed by strconv.ParseUint to fit in uint32 (due the bitSize argument) binary.LittleEndian.PutUint32(b, uint32(gatewayRaw)) return net.IP(b), nil