Skip to content

Commit

Permalink
Fix array index panic on SQL parsing (1.6 backport) (#856)
Browse files Browse the repository at this point in the history
* Fix array index panic on SQL parsing

* fix typo

* reformat code
  • Loading branch information
mariomac authored May 22, 2024
1 parent bd41788 commit 0d03f49
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 1 deletion.
17 changes: 16 additions & 1 deletion pkg/internal/ebpf/common/tcp_detect_transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func ReadTCPRequestIntoSpan(record *ringbuf.Record) (request.Span, bool, error)
}

func isSQL(buf string) int {
b := strings.ToUpper(buf)
b := asciiToUpper(buf)
for _, q := range []string{"SELECT", "UPDATE", "DELETE", "INSERT", "ALTER", "CREATE", "DROP"} {
i := strings.Index(b, q)
if i >= 0 {
Expand All @@ -53,6 +53,21 @@ func isSQL(buf string) int {
return -1
}

// when the input string is invalid unicode (might happen with the ringbuffer
// data), strings.ToUpper might return a string larger than the input string,
// and might cause some later out of bound errors.
func asciiToUpper(input string) string {
out := make([]byte, len(input))
for i := range input {
if input[i] >= 'a' && input[i] <= 'z' {
out[i] = input[i] - byte('a') + byte('A')
} else {
out[i] = input[i]
}
}
return string(out)
}

func (trace *TCPRequestInfo) reqHostInfo() (source, target string) {
src := make(net.IP, net.IPv6len)
dst := make(net.IP, net.IPv6len)
Expand Down
40 changes: 40 additions & 0 deletions pkg/internal/ebpf/common/tcp_detect_transform_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package ebpfcommon

import (
"bytes"
"encoding/binary"
"fmt"
"math/rand"
"strings"
"testing"

"github.com/cilium/ebpf/ringbuf"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/grafana/beyla/pkg/internal/request"
)
Expand Down Expand Up @@ -49,6 +53,42 @@ func TestSQLDetection(t *testing.T) {
}
}

// Test making sure that issue https://github.com/grafana/beyla/issues/854 is fixed
func TestReadTCPRequestIntoSpan_Overflow(t *testing.T) {
tri := TCPRequestInfo{
Len: 340,
// this byte array contains select * from foo
// rest of the array is invalid UTF-8 and would cause that strings.ToUpper
// returns a string longer than 256. That's why we are providing
// our own asciiToUpper implementation in isSQL function
Buf: [256]byte{
74, 39, 133, 207, 240, 83, 124, 225, 227, 163, 3, 23, 253, 254, 18, 12, 77, 143, 198, 122,
123, 67, 221, 225, 10, 233, 220, 36, 65, 35, 25, 251, 88, 197, 107, 99, 25, 247, 195, 216,
245, 107, 26, 144, 75, 78, 24, 70, 136, 173, 198, 79, 148, 232, 19, 253, 185, 169, 213, 97,
85, 119, 210, 114, 92, 26, 226, 241, 33, 16, 199, 78, 88, 108, 8, 211, 76, 188, 8, 170, 68,
128, 108, 194, 67, 240, 144, 132, 50, 191, 136, 130, 52, 210, 166, 212, 17, 179, 144, 138,
101, 98, 119, 16, 125, 99, 161, 176, 9, 25, 218, 236, 219, 22, 144, 91, 158, 146, 14, 243,
177, 58, 40, 139, 158, 33, 3, 91, 63, 70, 85, 20, 222, 206, 211, 152, 216, 53, 177, 125, 204,
219, 157, 151, 222, 184, 241, 193, 111, 22, 242, 185, 126, 159, 53, 181,
's', 'e', 'l', 'e', 'c', 't', ' ', '*', ' ', 'f', 'r', 'o', 'm', ' ', 'f', 'o', 'o',
0, 17, 111, 111, 133, 13, 221,
135, 126, 159, 234, 95, 233, 172, 96, 241, 140, 96, 71, 100, 223, 73, 74, 117, 239, 170, 154,
148, 167, 122, 215, 170, 51, 236, 146, 5, 61, 208, 74, 230, 243, 106, 222, 52, 138, 202, 39,
122, 180, 232, 43, 217, 86, 220, 38, 106, 141, 188, 27, 133, 156, 96, 107, 180, 178, 20, 62,
169, 193, 172, 206, 225, 219, 112, 52, 115, 32, 147, 192, 127, 211, 129, 241,
},
}
binaryRecord := bytes.Buffer{}
require.NoError(t, binary.Write(&binaryRecord, binary.LittleEndian, tri))
span, ignore, err := ReadTCPRequestIntoSpan(&ringbuf.Record{RawSample: binaryRecord.Bytes()})
require.NoError(t, err)
require.False(t, ignore)

assert.Equal(t, request.EventTypeSQLClient, span.Type)
assert.Equal(t, "SELECT", span.Method)
assert.Equal(t, "foo", span.Path)
}

const charset = "\\0\\1\\2abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"

func randomString(length int) string {
Expand Down

0 comments on commit 0d03f49

Please sign in to comment.