go/opentelemetry-go
go/opentelemetry-go
1
0
Fork 0
mirror of https://github.com/open-telemetry/opentelemetry-go.git synced 2024-11-21 16:46:38 +02:00
Code Issues Releases Activity

Fix gosec overflow alerts (#5799)

Browse Source 
To allow the golangci-lint upgrade in #5796.

---------

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
This commit is contained in:
Damien Mathieu 2024-09-13 09:11:50 +02:00 committed by GitHub
parent 5e3434c65a
commit a3c512aa95
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
26 changed files with 184 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
attribute/value_test.go
View File

@ -38,6 +38,12 @@ func TestValue(t *testing.T) {
wantType: attribute.INT64,
wantValue: int64(42),
},
{
name: "Key.Int64() correctly returns negative keys's internal int64 value",
value: k.Int64(-42).Value,
wantType: attribute.INT64,
wantValue: int64(-42),
},
{
name: "Key.Int64Slice() correctly returns keys's internal []int64 value",
value: k.Int64Slice([]int64{42, -3, 12}).Value,

6
bridge/opencensus/internal/ocmetric/metric.go
View File

@ -144,7 +144,7 @@ func convertHistogram(labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.Time
Attributes: attrs,
StartTime: t.StartTime,
Time: p.Time,
Count: uint64(dist.Count),
Count: uint64(max(0, dist.Count)), // nolint:gosec // A count should never be negative.
Sum: dist.Sum,
Bounds: dist.BucketOptions.Bounds,
BucketCounts: bucketCounts,
@ -166,7 +166,7 @@ func convertBuckets(buckets []ocmetricdata.Bucket) ([]uint64, []metricdata.Exemp
err = errors.Join(err, fmt.Errorf("%w: %q", errNegativeBucketCount, bucket.Count))
continue
}
bucketCounts[i] = uint64(bucket.Count)
bucketCounts[i] = uint64(max(0, bucket.Count)) // nolint:gosec // A count should never be negative.
if bucket.Exemplar != nil {
exemplar, exemplarErr := convertExemplar(bucket.Exemplar)
@ -357,7 +357,7 @@ func convertSummary(labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.TimeSe
Attributes: attrs,
StartTime: t.StartTime,
Time: p.Time,
Count: uint64(summary.Count),
Count: uint64(max(0, summary.Count)), // nolint:gosec // A count should never be negative.
QuantileValues: convertQuantiles(summary.Snapshot),
Sum: summary.Sum,
}

2
bridge/opencensus/internal/span.go
View File

@ -61,7 +61,7 @@ func (s *Span) SetName(name string) {
// SetStatus sets the status of this span, if it is recording events.
func (s *Span) SetStatus(status octrace.Status) {
s.otelSpan.SetStatus(codes.Code(status.Code), status.Message)
s.otelSpan.SetStatus(codes.Code(max(0, status.Code)), status.Message) // nolint:gosec // Overflow checked.
}
// AddAttributes sets attributes in this span.

41
bridge/opencensus/internal/span_test.go
View File

@ -96,15 +96,40 @@ func TestSpanSetStatus(t *testing.T) {
s := &span{recording: true}
ocS := internal.NewSpan(s)
c, d := codes.Error, "error"
status := octrace.Status{Code: int32(c), Message: d}
ocS.SetStatus(status)
for _, tt := range []struct {
name string
if s.sCode != c {
t.Error("span.SetStatus failed to set OpenTelemetry status code")
}
if s.sMsg != d {
t.Error("span.SetStatus failed to set OpenTelemetry status description")
code int32
message string
wantCode codes.Code
}{
{
name: "with an error code",
code: int32(codes.Error),
message: "error",
wantCode: codes.Error,
},
{
name: "with a negative/invalid code",
code: -42,
message: "error",
wantCode: codes.Unset,
},
} {
t.Run(tt.name, func(t *testing.T) {
status := octrace.Status{Code: tt.code, Message: tt.message}
ocS.SetStatus(status)
if s.sCode != tt.wantCode {
t.Errorf("span.SetStatus failed to set OpenTelemetry status code. Expected %d, got %d", tt.wantCode, s.sCode)
}
if s.sMsg != tt.message {
t.Errorf("span.SetStatus failed to set OpenTelemetry status description. Expected %s, got %s", tt.message, s.sMsg)
}
})
}
}

5
exporters/otlp/otlplog/otlploggrpc/internal/transform/log.go
View File

@ -139,10 +139,11 @@ func LogRecord(record log.Record) *lpb.LogRecord {
// year 1678 or after 2262). timeUnixNano on the zero Time returns 0. The
// result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
nano := t.UnixNano()
if nano < 0 {
return 0
}
return uint64(t.UnixNano())
return uint64(nano) // nolint:gosec // Overflow checked.
}
// AttrIter transforms an [attribute.Iterator] into OTLP key-values.

28
exporters/otlp/otlplog/otlploggrpc/internal/transform/log_test.go
View File

@ -30,6 +30,9 @@ var (
ts = time.Date(2000, time.January, 0o1, 0, 0, 0, 0, time.FixedZone("GMT", 0))
obs = ts.Add(30 * time.Second)
// A time before unix 0.
negativeTs = time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC)
alice = api.String("user", "alice")
bob = api.String("user", "bob")
@ -158,6 +161,20 @@ var (
Resource: res,
}.NewRecord())
out = append(out, logtest.RecordFactory{
Timestamp: negativeTs,
ObservedTimestamp: obs,
Severity: sevB,
SeverityText: "B",
Body: bodyB,
Attributes: []api.KeyValue{bob},
TraceID: trace.TraceID(traceIDB),
SpanID: trace.SpanID(spanIDB),
TraceFlags: trace.TraceFlags(flagsB),
InstrumentationScope: &scope,
Resource: res,
}.NewRecord())
return out
}()
@ -206,6 +223,17 @@ var (
TraceId: traceIDB,
SpanId: spanIDB,
},
{
TimeUnixNano: 0,
ObservedTimeUnixNano: uint64(obs.UnixNano()),
SeverityNumber: pbSevB,
SeverityText: "B",
Body: pbBodyB,
Attributes: []*cpb.KeyValue{pbBob},
Flags: uint32(flagsB),
TraceId: traceIDB,
SpanId: spanIDB,
},
}
pbScopeLogs = &lpb.ScopeLogs{

5
exporters/otlp/otlplog/otlploghttp/internal/transform/log.go
View File

@ -139,10 +139,11 @@ func LogRecord(record log.Record) *lpb.LogRecord {
// year 1678 or after 2262). timeUnixNano on the zero Time returns 0. The
// result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
nano := t.UnixNano()
if nano < 0 {
return 0
}
return uint64(t.UnixNano())
return uint64(nano) // nolint:gosec // Overflow checked.
}
// AttrIter transforms an [attribute.Iterator] into OTLP key-values.

28
exporters/otlp/otlplog/otlploghttp/internal/transform/log_test.go
View File

@ -30,6 +30,9 @@ var (
ts = time.Date(2000, time.January, 0o1, 0, 0, 0, 0, time.FixedZone("GMT", 0))
obs = ts.Add(30 * time.Second)
// A time before unix 0.
negativeTs = time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC)
alice = api.String("user", "alice")
bob = api.String("user", "bob")
@ -158,6 +161,20 @@ var (
Resource: res,
}.NewRecord())
out = append(out, logtest.RecordFactory{
Timestamp: negativeTs,
ObservedTimestamp: obs,
Severity: sevB,
SeverityText: "B",
Body: bodyB,
Attributes: []api.KeyValue{bob},
TraceID: trace.TraceID(traceIDB),
SpanID: trace.SpanID(spanIDB),
TraceFlags: trace.TraceFlags(flagsB),
InstrumentationScope: &scope,
Resource: res,
}.NewRecord())
return out
}()
@ -206,6 +223,17 @@ var (
TraceId: traceIDB,
SpanId: spanIDB,
},
{
TimeUnixNano: 0,
ObservedTimeUnixNano: uint64(obs.UnixNano()),
SeverityNumber: pbSevB,
SeverityText: "B",
Body: pbBodyB,
Attributes: []*cpb.KeyValue{pbBob},
Flags: uint32(flagsB),
TraceId: traceIDB,
SpanId: spanIDB,
},
}
pbScopeLogs = &lpb.ScopeLogs{

5
exporters/otlp/otlpmetric/otlpmetricgrpc/internal/transform/metricdata.go
View File

@ -279,10 +279,7 @@ func Temporality(t metricdata.Temporality) (mpb.AggregationTemporality, error) {
// timeUnixNano on the zero Time returns 0.
// The result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
return 0
}
return uint64(t.UnixNano())
return uint64(max(0, t.UnixNano())) // nolint:gosec // Overflow checked.
}
// Exemplars returns a slice of OTLP Exemplars generated from exemplars.

5
exporters/otlp/otlpmetric/otlpmetrichttp/internal/transform/metricdata.go
View File

@ -279,10 +279,7 @@ func Temporality(t metricdata.Temporality) (mpb.AggregationTemporality, error) {
// timeUnixNano on the zero Time returns 0.
// The result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
return 0
}
return uint64(t.UnixNano())
return uint64(max(0, t.UnixNano())) // nolint:gosec // Overflow checked.
}
// Exemplars returns a slice of OTLP Exemplars generated from exemplars.

8
exporters/otlp/otlptrace/internal/tracetransform/span.go
View File

@ -97,8 +97,8 @@ func span(sd tracesdk.ReadOnlySpan) *tracepb.Span {
SpanId: sid[:],
TraceState: sd.SpanContext().TraceState().String(),
Status: status(sd.Status().Code, sd.Status().Description),
StartTimeUnixNano: uint64(sd.StartTime().UnixNano()),
EndTimeUnixNano: uint64(sd.EndTime().UnixNano()),
StartTimeUnixNano: uint64(max(0, sd.StartTime().UnixNano())), // nolint:gosec // Overflow checked.
EndTimeUnixNano: uint64(max(0, sd.EndTime().UnixNano())), // nolint:gosec // Overflow checked.
Links: links(sd.Links()),
Kind: spanKind(sd.SpanKind()),
Name: sd.Name(),
@ -178,7 +178,7 @@ func buildSpanFlags(sc trace.SpanContext) uint32 {
flags |= tracepb.SpanFlags_SPAN_FLAGS_CONTEXT_IS_REMOTE_MASK
}
return uint32(flags)
return uint32(flags) // nolint:gosec // Flags is a bitmask and can't be negative
}
// spanEvents transforms span Events to an OTLP span events.
@ -192,7 +192,7 @@ func spanEvents(es []tracesdk.Event) []*tracepb.Span_Event {
for i := 0; i < len(es); i++ {
events[i] = &tracepb.Span_Event{
Name: es[i].Name,
TimeUnixNano: uint64(es[i].Time.UnixNano()),
TimeUnixNano: uint64(max(0, es[i].Time.UnixNano())), // nolint:gosec // Overflow checked.
Attributes: KeyValues(es[i].Attributes),
DroppedAttributesCount: clampUint32(es[i].DroppedAttributeCount),
}

10
exporters/otlp/otlptrace/internal/tracetransform/span_test.go
View File

@ -68,6 +68,7 @@ func TestEmptySpanEvent(t *testing.T) {
func TestSpanEvent(t *testing.T) {
attrs := []attribute.KeyValue{attribute.Int("one", 1), attribute.Int("two", 2)}
eventTime := time.Date(2020, 5, 20, 0, 0, 0, 0, time.UTC)
negativeEventTime := time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC)
got := spanEvents([]tracesdk.Event{
{
Name: "test 1",
@ -80,14 +81,21 @@ func TestSpanEvent(t *testing.T) {
Time: eventTime,
DroppedAttributeCount: 2,
},
{
Name: "test 3",
Attributes: attrs,
Time: negativeEventTime,
DroppedAttributeCount: 2,
},
})
if !assert.Len(t, got, 2) {
if !assert.Len(t, got, 3) {
return
}
eventTimestamp := uint64(1589932800 * 1e9)
assert.Equal(t, &tracepb.Span_Event{Name: "test 1", Attributes: nil, TimeUnixNano: eventTimestamp}, got[0])
// Do not test Attributes directly, just that the return value goes to the correct field.
assert.Equal(t, &tracepb.Span_Event{Name: "test 2", Attributes: KeyValues(attrs), TimeUnixNano: eventTimestamp, DroppedAttributesCount: 2}, got[1])
assert.Equal(t, &tracepb.Span_Event{Name: "test 3", Attributes: KeyValues(attrs), TimeUnixNano: 0, DroppedAttributesCount: 2}, got[2])
}
func TestNilLinks(t *testing.T) {

4
exporters/zipkin/internal/internaltest/text_map_propagator.go
View File

@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text
}
// InjectedN tests if p has made n injections to carrier.
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) {
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != n {
t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n)
return false
}

4
internal/internaltest/text_map_propagator.go
View File

@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text
}
// InjectedN tests if p has made n injections to carrier.
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) {
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != n {
t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n)
return false
}

3
internal/rawhelpers.go
View File

@ -20,7 +20,8 @@ func RawToBool(r uint64) bool {
}
func Int64ToRaw(i int64) uint64 {
return uint64(i)
// Assumes original was a valid int64 (overflow not checked).
return uint64(i) // nolint: gosec
}
func RawToInt64(r uint64) int64 {

4
internal/shared/internaltest/text_map_propagator.go.tmpl
View File

@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text
}
// InjectedN tests if p has made n injections to carrier.
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) {
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != n {
t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n)
return false
}

5
internal/shared/otlp/otlplog/transform/log.go.tmpl
View File

@ -139,10 +139,11 @@ func LogRecord(record log.Record) *lpb.LogRecord {
// year 1678 or after 2262). timeUnixNano on the zero Time returns 0. The
// result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
nano := t.UnixNano()
if nano < 0 {
return 0
}
return uint64(t.UnixNano())
return uint64(nano) // nolint:gosec // Overflow checked.
}
// AttrIter transforms an [attribute.Iterator] into OTLP key-values.

28
internal/shared/otlp/otlplog/transform/log_test.go.tmpl
View File

@ -30,6 +30,9 @@ var (
ts = time.Date(2000, time.January, 0o1, 0, 0, 0, 0, time.FixedZone("GMT", 0))
obs = ts.Add(30 * time.Second)
// A time before unix 0.
negativeTs = time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC)
alice = api.String("user", "alice")
bob = api.String("user", "bob")
@ -158,6 +161,20 @@ var (
Resource: res,
}.NewRecord())
out = append(out, logtest.RecordFactory{
Timestamp: negativeTs,
ObservedTimestamp: obs,
Severity: sevB,
SeverityText: "B",
Body: bodyB,
Attributes: []api.KeyValue{bob},
TraceID: trace.TraceID(traceIDB),
SpanID: trace.SpanID(spanIDB),
TraceFlags: trace.TraceFlags(flagsB),
InstrumentationScope: &scope,
Resource: res,
}.NewRecord())
return out
}()
@ -206,6 +223,17 @@ var (
TraceId: traceIDB,
SpanId: spanIDB,
},
{
TimeUnixNano: 0,
ObservedTimeUnixNano: uint64(obs.UnixNano()),
SeverityNumber: pbSevB,
SeverityText: "B",
Body: pbBodyB,
Attributes: []*cpb.KeyValue{pbBob},
Flags: uint32(flagsB),
TraceId: traceIDB,
SpanId: spanIDB,
},
}
pbScopeLogs = &lpb.ScopeLogs{

5
internal/shared/otlp/otlpmetric/transform/metricdata.go.tmpl
View File

@ -279,10 +279,7 @@ func Temporality(t metricdata.Temporality) (mpb.AggregationTemporality, error) {
// timeUnixNano on the zero Time returns 0.
// The result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
return 0
}
return uint64(t.UnixNano())
return uint64(max(0, t.UnixNano())) // nolint:gosec // Overflow checked.
}
// Exemplars returns a slice of OTLP Exemplars generated from exemplars.

3
log/keyvalue.go
View File

@ -76,7 +76,8 @@ func IntValue(v int) Value { return Int64Value(int64(v)) }
// Int64Value returns a [Value] for an int64.
func Int64Value(v int64) Value {
return Value{num: uint64(v), any: KindInt64}
// This can be later converted back to int64 (overflow not checked).
return Value{num: uint64(v), any: KindInt64} // nolint:gosec
}
// Float64Value returns a [Value] for a float64.

1
log/keyvalue_test.go
View File

@ -47,6 +47,7 @@ func TestValueEqual(t *testing.T) {
{},
log.Int64Value(1),
log.Int64Value(2),
log.Int64Value(-2),
log.Float64Value(3.5),
log.Float64Value(3.7),
log.BoolValue(true),

11
schema/internal/parser_checks.go
View File

@ -25,8 +25,15 @@ func CheckFileFormatField(fileFormat string, supportedFormatMajor, supportedForm
)
}
if supportedFormatMajor < 0 {
return errors.New("major version should be positive")
}
if supportedFormatMinor < 0 {
return errors.New("major version should be positive")
}
// Check that the major version number in the file is the same as what we expect.
if fileFormatParsed.Major() != uint64(supportedFormatMajor) {
if fileFormatParsed.Major() != uint64(supportedFormatMajor) { // nolint:gosec // Version can't be negative (overflow checked).
return fmt.Errorf(
"this library cannot parse file formats with major version other than %v",
supportedFormatMajor,
@ -35,7 +42,7 @@ func CheckFileFormatField(fileFormat string, supportedFormatMajor, supportedForm
// Check that the file minor version number is not greater than
// what is requested supports.
if fileFormatParsed.Minor() > uint64(supportedFormatMinor) {
if fileFormatParsed.Minor() > uint64(supportedFormatMinor) { // nolint:gosec // Version can't be negative (overflow checked).
supportedFormatMajorMinor := strconv.Itoa(supportedFormatMajor) + "." +
strconv.Itoa(supportedFormatMinor) // 1.0

2
schema/internal/parser_checks_test.go
View File

@ -14,6 +14,8 @@ func TestCheckFileFormatField(t *testing.T) {
assert.Error(t, CheckFileFormatField("not a semver", 1, 0))
assert.Error(t, CheckFileFormatField("2.0.0", 1, 0))
assert.Error(t, CheckFileFormatField("1.1.0", 1, 0))
assert.Error(t, CheckFileFormatField("1.1.0", -1, 0))
assert.Error(t, CheckFileFormatField("1.1.0", 1, -2))
assert.Error(t, CheckFileFormatField("1.2.0", 1, 1))

4
sdk/internal/internaltest/text_map_propagator.go
View File

@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text
}
// InjectedN tests if p has made n injections to carrier.
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) {
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != n {
t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n)
return false
}

3
sdk/metric/internal/exemplar/value.go
View File

@ -28,7 +28,8 @@ type Value struct {
func NewValue[N int64 | float64](value N) Value {
switch v := any(value).(type) {
case int64:
return Value{t: Int64ValueType, val: uint64(v)}
// This can be later converted back to int64 (overflow not checked).
return Value{t: Int64ValueType, val: uint64(v)} // nolint:gosec
case float64:
return Value{t: Float64ValueType, val: math.Float64bits(v)}
}

8
sdk/metric/internal/exemplar/value_test.go
View File

@ -10,8 +10,8 @@ import (
)
func TestValue(t *testing.T) {
const iVal, fVal = int64(43), float64(0.3)
i, f, bad := NewValue[int64](iVal), NewValue[float64](fVal), Value{}
const iVal, fVal, nVal = int64(43), float64(0.3), int64(-42)
i, f, n, bad := NewValue[int64](iVal), NewValue[float64](fVal), NewValue[int64](nVal), Value{}
assert.Equal(t, Int64ValueType, i.Type())
assert.Equal(t, iVal, i.Int64())
@ -21,6 +21,10 @@ func TestValue(t *testing.T) {
assert.Equal(t, fVal, f.Float64())
assert.Equal(t, int64(0), f.Int64())
assert.Equal(t, Int64ValueType, n.Type())
assert.Equal(t, nVal, n.Int64())
assert.Equal(t, float64(0), i.Float64())
assert.Equal(t, UnknownValueType, bad.Type())
assert.Equal(t, float64(0), bad.Float64())
assert.Equal(t, int64(0), bad.Int64())