Skip to content

Commit accc203

Browse files
authored
Merge pull request #212 from coroot/error_handling_in_ch_protocol_parsing
improve error handling in ClickHouse protocol parser
2 parents f56d905 + 6f5d688 commit accc203

File tree

2 files changed

+45
-2
lines changed

2 files changed

+45
-2
lines changed

ebpftracer/l7/clickhouse.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ func ParseClickhouse(payload []byte) string {
2121
return ""
2222
}
2323
if info.ProtocolVersion > 0 {
24+
if info.ProtocolVersion > version {
25+
return ""
26+
}
2427
version = info.ProtocolVersion
2528
}
2629
var s proto.Setting
@@ -36,10 +39,14 @@ func ParseClickhouse(payload []byte) string {
3639
if _, err = r.Str(); err != nil { // inter-server secret
3740
return ""
3841
}
39-
if _, err = r.UVarInt(); err != nil { // stage
42+
if stage, err := r.UVarInt(); err != nil { // stage
43+
return ""
44+
} else if stage > 2 { // invalid stage
4045
return ""
4146
}
42-
if _, err = r.UVarInt(); err != nil { // compression
47+
if c, err := r.UVarInt(); err != nil { // compression
48+
return ""
49+
} else if c > 1 { // invalid compression
4350
return ""
4451
}
4552
l, err := r.StrLen()

ebpftracer/l7/l7_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,42 @@ func TestParseClickHouse(t *testing.T) {
185185
`SELECT Timestamp, TraceId, SpanId, ParentSpanId, SpanName, ServiceName, Duration, StatusCode, StatusMessage, ResourceAttributes, SpanAttributes, Events.Timestamp, Events.Name, Events.Attributes FROM otel_traces_distributed WHERE ServiceName IN (['/k8s/coroot/coroot-coroot', '/system.slice/k3s-agent.service', '/system.slice/k3s.service']) AND (SpanAttributes['net.peer.name'] IN (['10.42.3.84', '10.42.1.73', '10.42.0.173', '10.42.5.69']) OR (SpanAttributes['net.peer.name'], SpanAttributes['net.peer.port']) IN (('10.42.3.84', '9009'), ('10.42.3.84', '0'), ('10.42.3.84', '9000'), ('10.42.3.84', '8123'), ('10.42.1.73', '0'), ('10.42.1.73', '8123'), ('10.42.1.73', '9009'), ('10.42.1.73', '9000'), ('10.42.0.173', '0'), ('10.42.0.173', '9009'), ('10.42.0.173', '9000'), ('10.42.0.173', '8123'), ('10.42.5.69', '9000'), ('10.42.5.69', '8123'), ('10.42.5.69', '9009'), ('10.42.5.69', '0'))) AND Tim...<TRUNCATED>`,
186186
ParseClickhouse(payload),
187187
)
188+
189+
payload = []byte{ // ClientQuerySecondary, parsing doesn't work
190+
0x01, 0x00, 0x02, 0x07, 0x64, 0x65, 0x66, 0x61, 0x75, 0x6c, 0x74, 0x24, 0x32, 0x34, 0x65, 0x61, 0x36, 0x33, 0x61,
191+
0x64, 0x2d, 0x32, 0x61, 0x34, 0x64, 0x2d, 0x34, 0x66, 0x32, 0x64, 0x2d, 0x38, 0x34, 0x65, 0x31, 0x2d, 0x34, 0x62,
192+
0x65, 0x62, 0x36, 0x35, 0x36, 0x61, 0x36, 0x30, 0x32, 0x32, 0x11, 0x31, 0x30, 0x2e, 0x34, 0x32, 0x2e, 0x30, 0x2e,
193+
0x32, 0x34, 0x33, 0x3a, 0x35, 0x38, 0x34, 0x37, 0x34, 0x78, 0xc3, 0x92, 0x37, 0xa3, 0x35, 0x06, 0x00, 0x01, 0x00,
194+
0x00, 0x10, 0x63, 0x6c, 0x69, 0x63, 0x6b, 0x68, 0x6f, 0x75, 0x73, 0x65, 0x2f, 0x63, 0x68, 0x2d, 0x67, 0x6f, 0x00,
195+
0x3e, 0xbc, 0xa9, 0x03, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0f, 0x73, 0x65, 0x6e, 0x64, 0x5f,
196+
0x6c, 0x6f, 0x67, 0x73, 0x5f, 0x6c, 0x65, 0x76, 0x65, 0x6c, 0x00, 0x04, 0x6e, 0x6f, 0x6e, 0x65, 0x00, 0x01, 0x00,
197+
0x00, 0x02, 0x01, 0xd0, 0x02, 0x49, 0x4e, 0x53, 0x45, 0x52, 0x54, 0x20, 0x49, 0x4e, 0x54, 0x4f, 0x20, 0x63, 0x6f,
198+
0x72, 0x6f, 0x6f, 0x74, 0x5f, 0x62, 0x31, 0x66, 0x35, 0x62, 0x77, 0x6b, 0x67, 0x2e, 0x6f, 0x74, 0x65, 0x6c, 0x5f,
199+
0x74, 0x72, 0x61, 0x63, 0x65, 0x73, 0x20, 0x28, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x2c, 0x20,
200+
0x54, 0x72, 0x61, 0x63, 0x65, 0x49, 0x64, 0x2c, 0x20, 0x53, 0x70, 0x61, 0x6e, 0x49, 0x64, 0x2c, 0x20, 0x50, 0x61,
201+
0x72, 0x65, 0x6e, 0x74, 0x53, 0x70, 0x61, 0x6e, 0x49, 0x64, 0x2c, 0x20, 0x54, 0x72, 0x61, 0x63, 0x65, 0x53, 0x74,
202+
0x61, 0x74, 0x65, 0x2c, 0x20, 0x53, 0x70, 0x61, 0x6e, 0x4e, 0x61, 0x6d, 0x65, 0x2c, 0x20, 0x53, 0x70, 0x61, 0x6e,
203+
0x4b, 0x69, 0x6e, 0x64, 0x2c, 0x20, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x4e, 0x61, 0x6d, 0x65, 0x2c, 0x20,
204+
0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x41, 0x74, 0x74, 0x72, 0x69, 0x62, 0x75, 0x74, 0x65, 0x73, 0x2c,
205+
0x20, 0x53, 0x70, 0x61, 0x6e, 0x41, 0x74, 0x74, 0x72, 0x69, 0x62, 0x75, 0x74, 0x65, 0x73, 0x2c, 0x20, 0x44, 0x75,
206+
0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2c, 0x20, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x43, 0x6f, 0x64, 0x65, 0x2c,
207+
0x20, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x2c, 0x20, 0x60, 0x45, 0x76,
208+
0x65, 0x6e, 0x74, 0x73, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x60, 0x2c, 0x20, 0x60, 0x45,
209+
0x76, 0x65, 0x6e, 0x74, 0x73, 0x2e, 0x4e, 0x61, 0x6d, 0x65, 0x60, 0x2c, 0x20, 0x60, 0x45, 0x76, 0x65, 0x6e, 0x74,
210+
0x73, 0x2e, 0x41, 0x74, 0x74, 0x72, 0x69, 0x62, 0x75, 0x74, 0x65, 0x73, 0x60, 0x2c, 0x20, 0x60, 0x4c, 0x69, 0x6e,
211+
0x6b, 0x73, 0x2e, 0x54, 0x72, 0x61, 0x63, 0x65, 0x49, 0x64, 0x60, 0x2c, 0x20, 0x60, 0x4c, 0x69, 0x6e, 0x6b, 0x73,
212+
0x2e, 0x53, 0x70, 0x61, 0x6e, 0x49, 0x64, 0x60, 0x2c, 0x20, 0x60, 0x4c, 0x69, 0x6e, 0x6b, 0x73, 0x2e, 0x54, 0x72,
213+
0x61, 0x63, 0x65, 0x53, 0x74, 0x61, 0x74, 0x65, 0x60, 0x2c, 0x20, 0x60, 0x4c, 0x69, 0x6e, 0x6b, 0x73, 0x2e, 0x41,
214+
0x74, 0x74, 0x72, 0x69, 0x62, 0x75, 0x74, 0x65, 0x73, 0x60, 0x29, 0x20, 0x56, 0x41, 0x4c, 0x55, 0x45, 0x53, 0x00,
215+
0x02, 0x00, 0xa7, 0x83, 0xac, 0x6c, 0xd5, 0x5c, 0x7a, 0x7c, 0xb5, 0xac, 0x46, 0xbd, 0xdb, 0x86, 0xe2, 0x14, 0x82,
216+
0x14, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0xa0, 0x01, 0x00, 0x02, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00,
217+
}
218+
219+
assert.Equal(t,
220+
``,
221+
ParseClickhouse(payload),
222+
)
223+
188224
}
189225

190226
func TestParseZookeeper(t *testing.T) {

0 commit comments

Comments
 (0)