From 3704bb5be3af7d2aa2ae3e82002df942d944bf86 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 22 Jul 2020 09:24:42 +0200 Subject: [PATCH 1/4] add defensive code on type assertion --- pkg/service/v0/service.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/service/v0/service.go b/pkg/service/v0/service.go index b5d56853ab..336537a65a 100644 --- a/pkg/service/v0/service.go +++ b/pkg/service/v0/service.go @@ -115,14 +115,15 @@ func getFailsafeIdentifier(c context.Context, identifier *proto.Identifier) *pro identifier = &proto.Identifier{} } if identifier.AccountUuid == "me" { - ownAccountUUID := c.Value(middleware.UUIDKey).(string) - if len(ownAccountUUID) > 0 { - identifier.AccountUuid = ownAccountUUID - } else { - // might be valid for the request not having an AccountUuid in the identifier. - // but clear it, instead of passing on `me`. - identifier.AccountUuid = "" + ownAccountUUID, ok := c.Value(middleware.UUIDKey).(string) + if ok { + if len(ownAccountUUID) > 0 { + identifier.AccountUuid = ownAccountUUID + } } + // might be valid for the request not having an AccountUuid in the identifier. + // but clear it, instead of passing on `me`. + identifier.AccountUuid = "" } identifier.AccountUuid = strings.ToLower(identifier.AccountUuid) return identifier From ca9e59c8b44b74fe8d5f83b293a170b5d94e70b0 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 22 Jul 2020 09:29:10 +0200 Subject: [PATCH 2/4] add changelog --- changelog/unreleased/hardening-type-assertion.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/hardening-type-assertion.md diff --git a/changelog/unreleased/hardening-type-assertion.md b/changelog/unreleased/hardening-type-assertion.md new file mode 100644 index 0000000000..1cd31f6d3b --- /dev/null +++ b/changelog/unreleased/hardening-type-assertion.md @@ -0,0 +1,6 @@ +Bugfix: Fix runtime error when type asserting on nil value + +Fixed the case where an account UUID present in the context is nil, and type asserting it as a string would produce a runtime error. + +https://github.com/owncloud/ocis-settings/pull/38 +https://github.com/owncloud/ocis-settings/issues/37 From 2c901cf16242a44119b5ac64e147f5ea854d6b60 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 22 Jul 2020 09:36:38 +0200 Subject: [PATCH 3/4] refactor --- pkg/service/v0/service.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/pkg/service/v0/service.go b/pkg/service/v0/service.go index 336537a65a..1fe5af3396 100644 --- a/pkg/service/v0/service.go +++ b/pkg/service/v0/service.go @@ -115,15 +115,9 @@ func getFailsafeIdentifier(c context.Context, identifier *proto.Identifier) *pro identifier = &proto.Identifier{} } if identifier.AccountUuid == "me" { - ownAccountUUID, ok := c.Value(middleware.UUIDKey).(string) - if ok { - if len(ownAccountUUID) > 0 { - identifier.AccountUuid = ownAccountUUID - } + if ownAccountUUID, ok := c.Value(middleware.UUIDKey).(string); ok { + identifier.AccountUuid = ownAccountUUID } - // might be valid for the request not having an AccountUuid in the identifier. - // but clear it, instead of passing on `me`. - identifier.AccountUuid = "" } identifier.AccountUuid = strings.ToLower(identifier.AccountUuid) return identifier From ea65b50000617252a646395189bfb815a82ca316 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 22 Jul 2020 10:08:14 +0200 Subject: [PATCH 4/4] unit test getFailsafeIdentifier --- pkg/service/v0/service_test.go | 65 ++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 pkg/service/v0/service_test.go diff --git a/pkg/service/v0/service_test.go b/pkg/service/v0/service_test.go new file mode 100644 index 0000000000..fa8b2381e5 --- /dev/null +++ b/pkg/service/v0/service_test.go @@ -0,0 +1,65 @@ +package svc + +import ( + "context" + "testing" + + "github.com/owncloud/ocis-pkg/v2/middleware" + "github.com/owncloud/ocis-settings/pkg/proto/v0" + "github.com/stretchr/testify/assert" +) + +var ( + ctxWithUUID = context.WithValue(context.Background(), middleware.UUIDKey, "61445573-4dbe-4d56-88dc-88ab47aceba7") + ctxWithEmptyUUID = context.WithValue(context.Background(), middleware.UUIDKey, "") + emptyCtx = context.Background() + + scenarios = []struct { + name string + identifier *proto.Identifier + ctx context.Context + expect *proto.Identifier + }{ + { + name: "context with UUID; identifier = 'me'", + ctx: ctxWithUUID, + identifier: &proto.Identifier{ + AccountUuid: "me", + }, + expect: &proto.Identifier{ + AccountUuid: ctxWithUUID.Value(middleware.UUIDKey).(string), + }, + }, + { + name: "context without UUID; identifier = 'me'", + ctx: ctxWithEmptyUUID, + identifier: &proto.Identifier{ + AccountUuid: "me", + }, + expect: &proto.Identifier{ + AccountUuid: "", + }, + }, + { + name: "context with UUID; identifier not 'me'", + ctx: ctxWithUUID, + identifier: &proto.Identifier{}, + expect: &proto.Identifier{ + AccountUuid: "", + }, + }, + } +) + +func TestGetFailsafeIdentifier(t *testing.T) { + for _, s := range scenarios { + scenario := s + t.Run(scenario.name, func(t *testing.T) { + got := getFailsafeIdentifier(scenario.ctx, scenario.identifier) + assert.NotPanics(t, func() { + getFailsafeIdentifier(emptyCtx, scenario.identifier) + }) + assert.Equal(t, scenario.expect, got) + }) + } +}