Skip to content

Commit 5e4905f

Browse files
committed
Fix panic in GenerateDeterministicNameFromSpec with long resolver names
GenerateDeterministicNameFromSpec panics when the generated name exceeds 63 characters (DNS-1123 label max length) because strings.LastIndex is used to find a space character as a truncation boundary. The generated name format is '{prefix}-{hex_hash}' which never contains spaces, so LastIndex returns -1, causing a slice bounds panic. An attacker who can create TaskRuns or PipelineRuns can set the resolver name to a string >= 30 characters to trigger this panic, crashing the controller into a CrashLoopBackOff. Fix: compute the hash first, then truncate only the prefix to make room, preserving the full hash to maintain determinism and uniqueness of generated ResolutionRequest names. Fixes: GHSA-cv4x-93xx-wgfj
1 parent 3f3e549 commit 5e4905f

2 files changed

Lines changed: 79 additions & 7 deletions

File tree

pkg/resolution/resource/name.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"hash"
2222
"hash/fnv"
2323
"sort"
24-
"strings"
2524

2625
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
2726
"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
@@ -77,6 +76,19 @@ func nameHasher() hash.Hash {
7776
return fnv.New128a()
7877
}
7978

79+
// sanitizedName builds a "{prefix}-{hash}" string that fits within the
80+
// DNS-1123 label max length. If the prefix is too long, it is truncated
81+
// so that the full hash is always preserved, maintaining determinism and
82+
// uniqueness.
83+
func sanitizedName(prefix string, hasher hash.Hash) string {
84+
hex := fmt.Sprintf("%x", hasher.Sum(nil))
85+
maxPrefixLen := maxLength - len(hex) - 1 // 1 for the "-" separator
86+
if len(prefix) > maxPrefixLen {
87+
prefix = prefix[:maxPrefixLen]
88+
}
89+
return fmt.Sprintf("%s-%s", prefix, hex)
90+
}
91+
8092
// GenerateDeterministicNameFromSpec makes a best-effort attempt to create a
8193
// unique but reproducible name for use in a Request. The returned value
8294
// will have the format {prefix}-{hash} where {prefix} is
@@ -89,7 +101,7 @@ func GenerateDeterministicNameFromSpec(prefix, base string, resolutionSpec *v1be
89101
}
90102

91103
if resolutionSpec == nil {
92-
return fmt.Sprintf("%s-%x", prefix, hasher.Sum(nil)), nil
104+
return sanitizedName(prefix, hasher), nil
93105
}
94106
params := resolutionSpec.Params
95107
sortedParams := make(v1.Params, len(params))
@@ -123,11 +135,7 @@ func GenerateDeterministicNameFromSpec(prefix, base string, resolutionSpec *v1be
123135
return "", err
124136
}
125137
}
126-
name := fmt.Sprintf("%s-%x", prefix, hasher.Sum(nil))
127-
if maxLength > len(name) {
128-
return name, nil
129-
}
130-
return name[:strings.LastIndex(name[:maxLength], " ")], nil
138+
return sanitizedName(prefix, hasher), nil
131139
}
132140

133141
// GenerateErrorLogString makes a best effort attempt to get the name of the task

pkg/resolution/resource/name_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,14 @@ func TestGenerateDeterministicName(t *testing.T) {
101101
args: golden,
102102
want: "prefix-ba2f256f318de7f4154da577c283cb9e",
103103
},
104+
{
105+
name: "long prefix with nil params truncates prefix preserving hash",
106+
args: args{
107+
prefix: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", // 31 chars
108+
base: "default/my-taskrun",
109+
},
110+
want: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-2d162955e9c9fcfef736fd389e7fd796",
111+
},
104112
}
105113
for _, tt := range tests {
106114
t.Run(tt.name, func(t *testing.T) {
@@ -201,6 +209,62 @@ func TestGenerateDeterministicNameFromSpec(t *testing.T) {
201209
args: golden,
202210
want: "prefix-ff25bd24688ab610bdc530a5ab3aabbd",
203211
},
212+
{
213+
name: "long prefix at exactly max length is not truncated",
214+
args: args{
215+
prefix: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", // 30 chars, name = 30+1+32 = 63 = maxLength
216+
base: "default/my-taskrun",
217+
params: []v1.Param{{
218+
Name: "foo",
219+
Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: "bar"},
220+
}},
221+
},
222+
want: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-da9d0a4501a276745547b4b4b21a2e77", // exactly 63, fits
223+
},
224+
{
225+
name: "long prefix exceeding max length truncates prefix not hash",
226+
args: args{
227+
prefix: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", // 31 chars, would be 64 > maxLength
228+
base: "default/my-taskrun",
229+
params: []v1.Param{{
230+
Name: "foo",
231+
Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: "bar"},
232+
}},
233+
},
234+
want: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-da9d0a4501a276745547b4b4b21a2e77", // prefix trimmed to 30, hash preserved
235+
},
236+
{
237+
name: "very long prefix truncates prefix preserving hash",
238+
args: args{
239+
prefix: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", // 50 chars
240+
base: "default/my-taskrun",
241+
},
242+
want: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-2d162955e9c9fcfef736fd389e7fd796", // prefix trimmed to 30, hash preserved
243+
},
244+
{
245+
name: "different inputs with same long prefix produce different names",
246+
args: args{
247+
prefix: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", // 31 chars
248+
base: "default/other-taskrun",
249+
params: []v1.Param{{
250+
Name: "baz",
251+
Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: "qux"},
252+
}},
253+
},
254+
want: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-5592e5f983640b4274071dbe1c09068d", // same prefix length, different hash
255+
},
256+
{
257+
name: "prefix under max length returns full name",
258+
args: args{
259+
prefix: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa", // 29 chars, name = 62 chars
260+
base: "default/my-taskrun",
261+
params: []v1.Param{{
262+
Name: "foo",
263+
Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: "bar"},
264+
}},
265+
},
266+
want: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa-da9d0a4501a276745547b4b4b21a2e77", // 62 chars, no truncation
267+
},
204268
}
205269
for _, tt := range tests {
206270
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)