Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix cast security issue #3243

Merged
merged 3 commits into from
Sep 29, 2024
Merged

fix: fix cast security issue #3243

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
334c25b
fix cast issue
Yisaer Sep 27, 2024
c71dc04
fix sec issue
Yisaer Sep 27, 2024
3762f80
fix
Yisaer Sep 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions internal/binder/function/funcs_math.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,9 @@ func conv(str string, fromBase, toBase int64) (res string, isNull bool, err erro
val = -val
}

if val > math.MaxInt64 {
return "", false, fmt.Errorf("value %d is out of int64 range", val)
}
if int64(val) < 0 {
negative = true
} else {
Expand Down
8 changes: 6 additions & 2 deletions internal/binder/function/funcs_math_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,8 @@ func TestConvFunc(t *testing.T) {
}{
{[]interface{}{"a", 16, 2}, "1010", false, false},
{[]interface{}{"6E", 18, 8}, "172", false, false},
{[]interface{}{"-17", 10, -18}, "-H", false, false},
{[]interface{}{"-17", 10, 18}, "2D3FGB0B9CG4BD1H", false, false},
{[]interface{}{"-17", 10, -18}, "-H", false, true},
{[]interface{}{"-17", 10, 18}, "2D3FGB0B9CG4BD1H", false, true},
{[]interface{}{"+18aZ", 7, 36}, "1", false, false},
{[]interface{}{"18446744073709551615", -10, 16}, "7FFFFFFFFFFFFFFF", false, false},
{[]interface{}{"12F", -10, 16}, "C", false, false},
Expand All @@ -559,6 +559,10 @@ func TestConvFunc(t *testing.T) {
}
for _, c := range cases {
got, _ := fConv.exec(fctx, []interface{}{c.args[0], c.args[1], c.args[2]})
if c.getErr {
require.Error(t, got.(error))
continue
}
if got != c.expected {
t.Errorf("%s:Expected %s, but got %s", c.args[0], c.expected, got)
}
Expand Down
13 changes: 13 additions & 0 deletions internal/converter/protobuf/fieldConverterSingleton.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import (
"fmt"
"math"

// TODO: replace with `google.golang.org/protobuf/proto` pkg.
"github.com/golang/protobuf/proto" //nolint:staticcheck
Expand Down Expand Up @@ -111,6 +112,9 @@
if err != nil {
return 0, nil
} else {
if r > math.MaxInt32 {
return 0, fmt.Errorf("value %d is out of int32 range", v)
}

Check warning on line 117 in internal/converter/protobuf/fieldConverterSingleton.go

View check run for this annotation

Codecov / codecov/patch

internal/converter/protobuf/fieldConverterSingleton.go#L116-L117 

Added lines #L116 - L117 were not covered by tests
return int32(r), nil
}
}, "int", cast.CONVERT_SAMEKIND)
Expand All @@ -122,6 +126,9 @@
if err != nil {
return 0, nil
} else {
if r > math.MaxUint32 {
return 0, fmt.Errorf("value %d is out of uint32 range", v)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above

}

Check warning on line 131 in internal/converter/protobuf/fieldConverterSingleton.go

View check run for this annotation

Codecov / codecov/patch

internal/converter/protobuf/fieldConverterSingleton.go#L130-L131 

Added lines #L130 - L131 were not covered by tests
return uint32(r), nil
}
}, "uint", cast.CONVERT_SAMEKIND)
Expand Down Expand Up @@ -174,6 +181,9 @@
case dpb.FieldDescriptorProto_TYPE_INT32, dpb.FieldDescriptorProto_TYPE_SFIXED32, dpb.FieldDescriptorProto_TYPE_SINT32, dpb.FieldDescriptorProto_TYPE_ENUM:
r, err := cast.ToInt(v, cast.CONVERT_SAMEKIND)
if err == nil {
if r > math.MaxInt32 {
return 0, fmt.Errorf("value %d is out of int32 range", v)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above

}

Check warning on line 186 in internal/converter/protobuf/fieldConverterSingleton.go

View check run for this annotation

Codecov / codecov/patch

internal/converter/protobuf/fieldConverterSingleton.go#L185-L186 

Added lines #L185 - L186 were not covered by tests
return int32(r), nil
} else {
return nil, fmt.Errorf("invalid type for int type field '%s': %v", fn, err)
Expand All @@ -188,6 +198,9 @@
case dpb.FieldDescriptorProto_TYPE_FIXED32, dpb.FieldDescriptorProto_TYPE_UINT32:
r, err := cast.ToUint64(v, cast.CONVERT_SAMEKIND)
if err == nil {
if r > math.MaxUint32 {
return 0, fmt.Errorf("value %d is out of uint32 range", v)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above

}

Check warning on line 203 in internal/converter/protobuf/fieldConverterSingleton.go

View check run for this annotation

Codecov / codecov/patch

internal/converter/protobuf/fieldConverterSingleton.go#L202-L203 

Added lines #L202 - L203 were not covered by tests
return uint32(r), nil
} else {
return nil, fmt.Errorf("invalid type for uint type field '%s': %v", fn, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/server/tracer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
if err != nil {
limit = 0
}
root, err := tracer.GetTraceIDListByRuleID(id, int(limit))
root, err := tracer.GetTraceIDListByRuleID(id, limit)

Check warning on line 77 in internal/server/tracer.go

View check run for this annotation

Codecov / codecov/patch

internal/server/tracer.go#L77 

Added line #L77 was not covered by tests
if err != nil {
handleError(w, err, "", logger)
return
Expand Down
16 changes: 16 additions & 0 deletions pkg/cast/cast.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import (
"encoding/base64"
"fmt"
"math"
"reflect"
"strconv"
"sync"
Expand Down Expand Up @@ -187,6 +188,9 @@
if sn == CONVERT_ALL {
v, err := strconv.ParseInt(s, 0, 0)
if err == nil {
if v > math.MaxInt8 {
return 0, fmt.Errorf("value %d is out of int8 range", v)
}

Check warning on line 193 in pkg/cast/cast.go

View check run for this annotation

Codecov / codecov/patch

pkg/cast/cast.go#L192-L193 

Added lines #L192 - L193 were not covered by tests
return int8(v), nil
}
}
Expand Down Expand Up @@ -239,6 +243,9 @@
if sn == CONVERT_ALL {
v, err := strconv.ParseInt(s, 0, 0)
if err == nil {
if v > math.MaxInt16 {
return 0, fmt.Errorf("value %d is out of int32 range", v)
}

Check warning on line 248 in pkg/cast/cast.go

View check run for this annotation

Codecov / codecov/patch

pkg/cast/cast.go#L247-L248 

Added lines #L247 - L248 were not covered by tests
return int16(v), nil
}
}
Expand Down Expand Up @@ -291,6 +298,9 @@
if sn == CONVERT_ALL {
v, err := strconv.ParseInt(s, 0, 0)
if err == nil {
if v > math.MaxInt32 {
return 0, fmt.Errorf("value %d is out of int32 range", v)
}

Check warning on line 303 in pkg/cast/cast.go

View check run for this annotation

Codecov / codecov/patch

pkg/cast/cast.go#L302-L303 

Added lines #L302 - L303 were not covered by tests
return int32(v), nil
}
}
Expand Down Expand Up @@ -577,6 +587,9 @@
if sn == CONVERT_ALL {
v, err := strconv.ParseUint(s, 0, 64)
if err == nil {
if v > math.MaxUint8 {
return 0, fmt.Errorf("value %d is out of uint16 range", v)
}

Check warning on line 592 in pkg/cast/cast.go

View check run for this annotation

Codecov / codecov/patch

pkg/cast/cast.go#L591-L592 

Added lines #L591 - L592 were not covered by tests
return uint8(v), nil
}
}
Expand Down Expand Up @@ -650,6 +663,9 @@
if sn == CONVERT_ALL {
v, err := strconv.ParseUint(s, 0, 64)
if err == nil {
if v > math.MaxUint16 {
return 0, fmt.Errorf("value %d is out of uint16 range", v)
}

Check warning on line 668 in pkg/cast/cast.go

View check run for this annotation

Codecov / codecov/patch

pkg/cast/cast.go#L667-L668 

Added lines #L667 - L668 were not covered by tests
return uint16(v), nil
}
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/tracer/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,14 @@
return l.spanStorage.GetTraceById(traceID)
}

func (l *SpanExporter) GetTraceByRuleID(ruleID string, limit int) ([]string, error) {
func (l *SpanExporter) GetTraceByRuleID(ruleID string, limit int64) ([]string, error) {

Check warning on line 96 in pkg/tracer/manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/tracer/manager.go#L96 

Added line #L96 was not covered by tests
return l.spanStorage.GetTraceByRuleID(ruleID, limit)
}

type LocalSpanStorage interface {
SaveSpan(span sdktrace.ReadOnlySpan) error
GetTraceById(traceID string) (*LocalSpan, error)
GetTraceByRuleID(ruleID string, limit int) ([]string, error)
GetTraceByRuleID(ruleID string, limit int64) ([]string, error)
}

type LocalSpanMemoryStorage struct {
Expand Down Expand Up @@ -169,15 +169,15 @@
return rootSpan, nil
}

func (l *LocalSpanMemoryStorage) GetTraceByRuleID(ruleID string, limit int) ([]string, error) {
func (l *LocalSpanMemoryStorage) GetTraceByRuleID(ruleID string, limit int64) ([]string, error) {
l.RLock()
defer l.RUnlock()
traceMap := l.ruleTraceMap[ruleID]
r := make([]string, 0)
if limit < 1 {
limit = len(traceMap)
limit = int64(len(traceMap))
}
count := 0
count := int64(0)
for traceID := range traceMap {
r = append(r, traceID)
count++
Expand Down Expand Up @@ -291,7 +291,7 @@
return s.loadTraceByTraceID(traceID)
}

func (s *sqlSpanStorage) GetTraceByRuleID(ruleID string, limit int) ([]string, error) {
func (s *sqlSpanStorage) GetTraceByRuleID(ruleID string, limit int64) ([]string, error) {

Check warning on line 294 in pkg/tracer/manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/tracer/manager.go#L294 

Added line #L294 was not covered by tests
return s.loadTraceByRuleID(ruleID)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/tracer/tracer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
return g.SpanExporter.GetTraceById(traceID)
}

func (g *GlobalTracerManager) GetTraceByRuleID(ruleID string, limit int) ([]string, error) {
func (g *GlobalTracerManager) GetTraceByRuleID(ruleID string, limit int64) ([]string, error) {

Check warning on line 97 in pkg/tracer/tracer.go

View check run for this annotation

Codecov / codecov/patch

pkg/tracer/tracer.go#L97 

Added line #L97 was not covered by tests
g.RLock()
defer g.RUnlock()
return g.SpanExporter.GetTraceByRuleID(ruleID, limit)
Expand Down Expand Up @@ -145,7 +145,7 @@
return tracerConfig, nil
}

func GetTraceIDListByRuleID(ruleID string, limit int) ([]string, error) {
func GetTraceIDListByRuleID(ruleID string, limit int64) ([]string, error) {

Check warning on line 148 in pkg/tracer/tracer.go

View check run for this annotation

Codecov / codecov/patch

pkg/tracer/tracer.go#L148 

Added line #L148 was not covered by tests
globalTracerManager.InitIfNot()
return globalTracerManager.GetTraceByRuleID(ruleID, limit)
}
Loading