From 13f40848e96d4211679ec4cadd0e93dc028a4f12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Th=C3=B6mmes?= Date: Tue, 16 Feb 2021 11:17:26 +0100 Subject: [PATCH] Enable gosec linter and fix existing issues (#1228) * Enable gosec linter * Fix issues with pointers to loop variables * Comment on potential security issues --- .golangci.yaml | 4 +++- pkg/dynamic/client.go | 7 ++++--- pkg/kn/commands/broker/list.go | 5 +++-- pkg/kn/commands/channel/flags.go | 12 +++++++----- pkg/kn/commands/revision/human_readable_flags.go | 5 +++-- pkg/kn/commands/route/human_readable_flags.go | 5 +++-- pkg/kn/commands/service/describe.go | 3 ++- pkg/kn/commands/service/human_readable_flags.go | 5 +++-- pkg/kn/commands/source/apiserver/flags.go | 12 +++++++----- pkg/kn/commands/source/binding/flags.go | 5 +++-- .../source/container/human_readable_flags.go | 12 +++++++----- pkg/kn/commands/source/duck/multisourcelist.go | 5 +++-- pkg/kn/commands/source/human_readable_flags.go | 5 +++-- pkg/kn/commands/source/ping/flags.go | 12 +++++++----- pkg/kn/commands/subscription/flags.go | 12 +++++++----- pkg/kn/commands/trigger/list_flags.go | 12 +++++++----- pkg/kn/flags/podspec_helper.go | 5 +++-- pkg/kn/plugin/manager.go | 1 + pkg/serving/service.go | 1 + pkg/util/corev1_helper.go | 3 ++- 20 files changed, 79 insertions(+), 52 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index cece74ca9e..864b563e2d 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -7,9 +7,10 @@ run: linters: enable: - errorlint + - gosec + - prealloc - unconvert - unparam - - prealloc disable: - errcheck @@ -17,4 +18,5 @@ issues: exclude-rules: - path: test # Excludes /test, *_test.go etc. linters: + - gosec - unparam diff --git a/pkg/dynamic/client.go b/pkg/dynamic/client.go index daa0a8f2f0..d7b6789e7a 100644 --- a/pkg/dynamic/client.go +++ b/pkg/dynamic/client.go @@ -165,9 +165,10 @@ func (c *knDynamicClient) ListSources(types ...WithType) (*unstructured.Unstruct namespace := c.Namespace() filters := WithTypes(types).List() // For each source type available, find out each source types objects - for _, source := range sourceTypes.Items { + for i := range sourceTypes.Items { + source := &sourceTypes.Items[i] // find source kind before hand to fail early - sourceKind, err := kindFromUnstructured(&source) + sourceKind, err := kindFromUnstructured(source) if err != nil { return nil, err } @@ -177,7 +178,7 @@ func (c *knDynamicClient) ListSources(types ...WithType) (*unstructured.Unstruct } // find source's GVR from unstructured source type object - gvr, err := gvrFromUnstructured(&source) + gvr, err := gvrFromUnstructured(source) if err != nil { return nil, err } diff --git a/pkg/kn/commands/broker/list.go b/pkg/kn/commands/broker/list.go index d5af952061..3693913c50 100644 --- a/pkg/kn/commands/broker/list.go +++ b/pkg/kn/commands/broker/list.go @@ -102,8 +102,9 @@ func ListHandlers(h hprinters.PrintHandler) { func printBrokerList(kServiceList *v1beta1.BrokerList, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { rows := make([]metav1beta1.TableRow, 0, len(kServiceList.Items)) - for _, ksvc := range kServiceList.Items { - r, err := printBroker(&ksvc, options) + for i := range kServiceList.Items { + ksvc := &kServiceList.Items[i] + r, err := printBroker(ksvc, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/channel/flags.go b/pkg/kn/commands/channel/flags.go index 633de5af37..dfcb5941f5 100644 --- a/pkg/kn/commands/channel/flags.go +++ b/pkg/kn/commands/channel/flags.go @@ -85,8 +85,9 @@ func printChannelList(channelList *messagingv1beta1.ChannelList, options hprinte return channelList.Items[i].GetName() < channelList.Items[j].GetName() }) - for _, item := range channelList.Items { - row, err := printChannel(&item, options) + for i := range channelList.Items { + item := &channelList.Items[i] + row, err := printChannel(item, options) if err != nil { return nil, err } @@ -103,10 +104,11 @@ func printChannelListWithNamespace(channelList *messagingv1beta1.ChannelList, op // temporary slice for sorting services in non-default namespace others := make([]metav1beta1.TableRow, 0, len(rows)) - for _, channel := range channelList.Items { + for i := range channelList.Items { + channel := &channelList.Items[i] // Fill in with services in `default` namespace at first if channel.Namespace == "default" { - r, err := printChannel(&channel, options) + r, err := printChannel(channel, options) if err != nil { return nil, err } @@ -114,7 +116,7 @@ func printChannelListWithNamespace(channelList *messagingv1beta1.ChannelList, op continue } // put other services in temporary slice - r, err := printChannel(&channel, options) + r, err := printChannel(channel, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/revision/human_readable_flags.go b/pkg/kn/commands/revision/human_readable_flags.go index edcc37d248..081279b9c4 100644 --- a/pkg/kn/commands/revision/human_readable_flags.go +++ b/pkg/kn/commands/revision/human_readable_flags.go @@ -56,8 +56,9 @@ func RevisionListHandlers(h hprinters.PrintHandler) { func printRevisionList(revisionList *servingv1.RevisionList, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { rows := make([]metav1beta1.TableRow, 0, len(revisionList.Items)) - for _, rev := range revisionList.Items { - r, err := printRevision(&rev, options) + for i := range revisionList.Items { + rev := &revisionList.Items[i] + r, err := printRevision(rev, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/route/human_readable_flags.go b/pkg/kn/commands/route/human_readable_flags.go index 4886816c43..f366f9580d 100644 --- a/pkg/kn/commands/route/human_readable_flags.go +++ b/pkg/kn/commands/route/human_readable_flags.go @@ -37,8 +37,9 @@ func RouteListHandlers(h hprinters.PrintHandler) { // printKRouteList populates the Knative route list table rows func printKRouteList(kRouteList *servingv1.RouteList, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { rows := make([]metav1beta1.TableRow, 0, len(kRouteList.Items)) - for _, ksvc := range kRouteList.Items { - r, err := printRoute(&ksvc, options) + for i := range kRouteList.Items { + ksvc := &kRouteList.Items[i] + r, err := printRoute(ksvc, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/service/describe.go b/pkg/kn/commands/service/describe.go index 713a279d63..2b15f13968 100644 --- a/pkg/kn/commands/service/describe.go +++ b/pkg/kn/commands/service/describe.go @@ -280,7 +280,8 @@ func getRevisionDescriptions(client clientservingv1.KnServingClient, service *se trafficTargets := service.Status.Traffic var err error - for _, target := range trafficTargets { + for i := range trafficTargets { + target := trafficTargets[i] revision, err := extractRevisionFromTarget(client, target) if err != nil { return nil, fmt.Errorf("cannot extract revision from service %s: %w", service.Name, err) diff --git a/pkg/kn/commands/service/human_readable_flags.go b/pkg/kn/commands/service/human_readable_flags.go index 281678d1b3..a4e2a8fb79 100644 --- a/pkg/kn/commands/service/human_readable_flags.go +++ b/pkg/kn/commands/service/human_readable_flags.go @@ -46,8 +46,9 @@ func ServiceListHandlers(h hprinters.PrintHandler) { func printKServiceList(kServiceList *servingv1.ServiceList, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { rows := make([]metav1beta1.TableRow, 0, len(kServiceList.Items)) - for _, ksvc := range kServiceList.Items { - r, err := printKService(&ksvc, options) + for i := range kServiceList.Items { + ksvc := &kServiceList.Items[i] + r, err := printKService(ksvc, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/source/apiserver/flags.go b/pkg/kn/commands/source/apiserver/flags.go index da525fa826..552a2da15c 100644 --- a/pkg/kn/commands/source/apiserver/flags.go +++ b/pkg/kn/commands/source/apiserver/flags.go @@ -260,8 +260,9 @@ func printSourceList(sourceList *v1alpha2.ApiServerSourceList, options hprinters return sourceList.Items[i].GetName() < sourceList.Items[j].GetName() }) - for _, item := range sourceList.Items { - row, err := printSource(&item, options) + for i := range sourceList.Items { + item := &sourceList.Items[i] + row, err := printSource(item, options) if err != nil { return nil, err } @@ -278,10 +279,11 @@ func printSourceListWithNamespace(sourceList *v1alpha2.ApiServerSourceList, opti // temporary slice for sorting services in non-default namespace others := []metav1beta1.TableRow{} - for _, source := range sourceList.Items { + for i := range sourceList.Items { + source := &sourceList.Items[i] // Fill in with services in `default` namespace at first if source.Namespace == "default" { - r, err := printSource(&source, options) + r, err := printSource(source, options) if err != nil { return nil, err } @@ -289,7 +291,7 @@ func printSourceListWithNamespace(sourceList *v1alpha2.ApiServerSourceList, opti continue } // put other services in temporary slice - r, err := printSource(&source, options) + r, err := printSource(source, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/source/binding/flags.go b/pkg/kn/commands/source/binding/flags.go index 013b4bb38b..10072aafbf 100644 --- a/pkg/kn/commands/source/binding/flags.go +++ b/pkg/kn/commands/source/binding/flags.go @@ -85,8 +85,9 @@ func printSinkBinding(binding *v1alpha2.SinkBinding, options hprinters.PrintOpti func printSinkBindingList(sinkBindingList *v1alpha2.SinkBindingList, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { rows := make([]metav1beta1.TableRow, 0, len(sinkBindingList.Items)) - for _, binding := range sinkBindingList.Items { - r, err := printSinkBinding(&binding, options) + for i := range sinkBindingList.Items { + binding := &sinkBindingList.Items[i] + r, err := printSinkBinding(binding, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/source/container/human_readable_flags.go b/pkg/kn/commands/source/container/human_readable_flags.go index bdd35f9745..a22ab20cf7 100644 --- a/pkg/kn/commands/source/container/human_readable_flags.go +++ b/pkg/kn/commands/source/container/human_readable_flags.go @@ -85,8 +85,9 @@ func printSourceList(sourceList *v1alpha2.ContainerSourceList, options hprinters return sourceList.Items[i].GetName() < sourceList.Items[j].GetName() }) - for _, item := range sourceList.Items { - row, err := printSource(&item, options) + for i := range sourceList.Items { + item := &sourceList.Items[i] + row, err := printSource(item, options) if err != nil { return nil, err } @@ -103,10 +104,11 @@ func printSourceListWithNamespace(sourceList *v1alpha2.ContainerSourceList, opti // temporary slice for sorting services in non-default namespace others := []metav1beta1.TableRow{} - for _, source := range sourceList.Items { + for i := range sourceList.Items { + source := &sourceList.Items[i] // Fill in with services in `default` namespace at first if source.Namespace == "default" { - r, err := printSource(&source, options) + r, err := printSource(source, options) if err != nil { return nil, err } @@ -114,7 +116,7 @@ func printSourceListWithNamespace(sourceList *v1alpha2.ContainerSourceList, opti continue } // put other services in temporary slice - r, err := printSource(&source, options) + r, err := printSource(source, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/source/duck/multisourcelist.go b/pkg/kn/commands/source/duck/multisourcelist.go index aa17067e7a..3b06aa7059 100644 --- a/pkg/kn/commands/source/duck/multisourcelist.go +++ b/pkg/kn/commands/source/duck/multisourcelist.go @@ -89,8 +89,9 @@ func toSource(u *unstructured.Unstructured) Source { func ToSourceList(uList *unstructured.UnstructuredList) *SourceList { dsl := SourceList{Items: []Source{}} //dsl.Items = make(Source, 0, len(uList.Items)) - for _, u := range uList.Items { - dsl.Items = append(dsl.Items, toSource(&u)) + for i := range uList.Items { + u := &uList.Items[i] + dsl.Items = append(dsl.Items, toSource(u)) } // set empty group, version and non empty kind dsl.APIVersion, dsl.Kind = schema.GroupVersion{}.WithKind(DSListKind).ToAPIVersionAndKind() diff --git a/pkg/kn/commands/source/human_readable_flags.go b/pkg/kn/commands/source/human_readable_flags.go index ac0579382e..f511948abe 100644 --- a/pkg/kn/commands/source/human_readable_flags.go +++ b/pkg/kn/commands/source/human_readable_flags.go @@ -127,8 +127,9 @@ func printSourceList(sourceList *clientduck.SourceList, options printers.PrintOp sort.SliceStable(sourceList.Items, func(i, j int) bool { return sourceList.Items[i].Name < sourceList.Items[j].Name }) - for _, source := range sourceList.Items { - row, err := printSource(&source, options) + for i := range sourceList.Items { + source := &sourceList.Items[i] + row, err := printSource(source, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/source/ping/flags.go b/pkg/kn/commands/source/ping/flags.go index 626d37b8d4..212b2dcfa3 100644 --- a/pkg/kn/commands/source/ping/flags.go +++ b/pkg/kn/commands/source/ping/flags.go @@ -112,8 +112,9 @@ func printSourceList(sourceList *v1alpha2.PingSourceList, options hprinters.Prin return sourceList.Items[i].GetName() < sourceList.Items[j].GetName() }) - for _, item := range sourceList.Items { - row, err := printSource(&item, options) + for i := range sourceList.Items { + item := &sourceList.Items[i] + row, err := printSource(item, options) if err != nil { return nil, err } @@ -130,10 +131,11 @@ func printSourceListWithNamespace(sourceList *v1alpha2.PingSourceList, options h // temporary slice for sorting services in non-default namespace others := make([]metav1beta1.TableRow, 0, len(rows)) - for _, source := range sourceList.Items { + for i := range sourceList.Items { + source := &sourceList.Items[i] // Fill in with services in `default` namespace at first if source.Namespace == "default" { - r, err := printSource(&source, options) + r, err := printSource(source, options) if err != nil { return nil, err } @@ -141,7 +143,7 @@ func printSourceListWithNamespace(sourceList *v1alpha2.PingSourceList, options h continue } // put other services in temporary slice - r, err := printSource(&source, options) + r, err := printSource(source, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/subscription/flags.go b/pkg/kn/commands/subscription/flags.go index 88dfc2df3f..f7cf3c0f77 100644 --- a/pkg/kn/commands/subscription/flags.go +++ b/pkg/kn/commands/subscription/flags.go @@ -94,8 +94,9 @@ func printSubscriptionList(subscriptionList *messagingv1beta1.SubscriptionList, return subscriptionList.Items[i].GetName() < subscriptionList.Items[j].GetName() }) - for _, item := range subscriptionList.Items { - row, err := printSubscription(&item, options) + for i := range subscriptionList.Items { + item := &subscriptionList.Items[i] + row, err := printSubscription(item, options) if err != nil { return nil, err } @@ -112,10 +113,11 @@ func printSubscriptionListWithNamespace(subscriptionList *messagingv1beta1.Subsc // temporary slice for sorting services in non-default namespace others := make([]metav1beta1.TableRow, 0, len(rows)) - for _, subscription := range subscriptionList.Items { + for i := range subscriptionList.Items { + subscription := &subscriptionList.Items[i] // Fill in with services in `default` namespace at first if subscription.Namespace == "default" { - r, err := printSubscription(&subscription, options) + r, err := printSubscription(subscription, options) if err != nil { return nil, err } @@ -123,7 +125,7 @@ func printSubscriptionListWithNamespace(subscriptionList *messagingv1beta1.Subsc continue } // put other services in temporary slice - r, err := printSubscription(&subscription, options) + r, err := printSubscription(subscription, options) if err != nil { return nil, err } diff --git a/pkg/kn/commands/trigger/list_flags.go b/pkg/kn/commands/trigger/list_flags.go index 9a8496f082..0dbf6fb4df 100644 --- a/pkg/kn/commands/trigger/list_flags.go +++ b/pkg/kn/commands/trigger/list_flags.go @@ -77,10 +77,11 @@ func printTriggerListWithNamespace(triggerList *v1beta1.TriggerList, options hpr // temporary slice for sorting services in non-default namespace others := []metav1beta1.TableRow{} - for _, trigger := range triggerList.Items { + for i := range triggerList.Items { + trigger := &triggerList.Items[i] // Fill in with services in `default` namespace at first if trigger.Namespace == "default" { - r, err := printTrigger(&trigger, options) + r, err := printTrigger(trigger, options) if err != nil { return nil, err } @@ -88,7 +89,7 @@ func printTriggerListWithNamespace(triggerList *v1beta1.TriggerList, options hpr continue } // put other services in temporary slice - r, err := printTrigger(&trigger, options) + r, err := printTrigger(trigger, options) if err != nil { return nil, err } @@ -111,8 +112,9 @@ func printTriggerList(triggerList *v1beta1.TriggerList, options hprinters.PrintO return printTriggerListWithNamespace(triggerList, options) } - for _, trigger := range triggerList.Items { - r, err := printTrigger(&trigger, options) + for i := range triggerList.Items { + trigger := &triggerList.Items[i] + r, err := printTrigger(trigger, options) if err != nil { return nil, err } diff --git a/pkg/kn/flags/podspec_helper.go b/pkg/kn/flags/podspec_helper.go index f9b19d65fb..9bcb15c0b7 100644 --- a/pkg/kn/flags/podspec_helper.go +++ b/pkg/kn/flags/podspec_helper.go @@ -249,8 +249,9 @@ func removeEnvVars(env []corev1.EnvVar, toRemove []string) []corev1.EnvVar { func updateEnvFrom(envFromSources []corev1.EnvFromSource, toUpdate []string) ([]corev1.EnvFromSource, error) { existingNameSet := make(map[string]bool) - for _, envSrc := range envFromSources { - if canonicalName, err := getCanonicalNameFromEnvFromSource(&envSrc); err == nil { + for i := range envFromSources { + envSrc := &envFromSources[i] + if canonicalName, err := getCanonicalNameFromEnvFromSource(envSrc); err == nil { existingNameSet[canonicalName] = true } } diff --git a/pkg/kn/plugin/manager.go b/pkg/kn/plugin/manager.go index 98b0179c30..0ecb72f1a9 100644 --- a/pkg/kn/plugin/manager.go +++ b/pkg/kn/plugin/manager.go @@ -232,6 +232,7 @@ func (manager *Manager) LookupInPath() bool { // Execute the plugin with the given arguments func (plugin *plugin) Execute(args []string) error { + //nolint:gosec // Passing the arguments through is expected, the plugins are trusted. cmd := exec.Command(plugin.path, args...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/pkg/serving/service.go b/pkg/serving/service.go index 83f08242c6..306dc0d304 100644 --- a/pkg/serving/service.go +++ b/pkg/serving/service.go @@ -36,6 +36,7 @@ type revisionTemplContext struct { func (c *revisionTemplContext) Random(l int) string { chars := make([]string, 0, l) for i := 0; i < l; i++ { + //nolint:gosec // Weak crypto is fine here, we use it for generating unique keys. chars = append(chars, charChoices[rand.Int()%len(charChoices)]) } return strings.Join(chars, "") diff --git a/pkg/util/corev1_helper.go b/pkg/util/corev1_helper.go index 22f23f2df2..d0a08892cc 100644 --- a/pkg/util/corev1_helper.go +++ b/pkg/util/corev1_helper.go @@ -17,7 +17,7 @@ limitations under the License. package util import ( - "crypto/sha1" + "crypto/sha1" //nolint:gosec // Weak crypto is fine here, we use it for generating unique keys. "fmt" "strings" "unicode" @@ -81,6 +81,7 @@ func GenerateVolumeName(path string) string { } func appendCheckSum(sanitizedString, path string) string { + //nolint:gosec // Weak crypto is fine here, we use it for generating unique keys. checkSum := sha1.Sum([]byte(path)) shortCheckSum := checkSum[0:4] return fmt.Sprintf("%s-%x", sanitizedString, shortCheckSum)