Skip to content

Commit ed2d884

Browse files
fearful-symmetryfearful-symmetry
authored
Add check for incorrect length values in postgres DataRow parser (#47872)
* add check for incorrect length values in postgres DataRow parser * add changelog * linter... * linter.... * fix log message
1 parent 56a35a9 commit ed2d884

File tree

3 files changed

+138
-18
lines changed

3 files changed

+138
-18
lines changed

‎changelog/fragments/1764705900-add-check-for-incorrect-length-values-in-postgres-datarow-parser.yaml‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# REQUIRED
2+
# Kind can be one of:
3+
# - breaking-change: a change to previously-documented behavior
4+
# - deprecation: functionality that is being removed in a later release
5+
# - bug-fix: fixes a problem in a previous version
6+
# - enhancement: extends functionality but does not break or fix existing behavior
7+
# - feature: new functionality
8+
# - known-issue: problems that we are aware of in a given version
9+
# - security: impacts on the security of a product or a user’s deployment.
10+
# - upgrade: important information for someone upgrading from a prior version
11+
# - other: does not fit into any of the other categories
12+
kind: bug-fix
13+
14+
# REQUIRED for all kinds
15+
# Change summary; a 80ish characters long description of the change.
16+
summary: Add check for incorrect length values in postgres datarow parser
17+
18+
# REQUIRED for breaking-change, deprecation, known-issue
19+
# Long description; in case the summary is not enough to describe the change
20+
# this field accommodate a description without length limits.
21+
# description:
22+
23+
# REQUIRED for breaking-change, deprecation, known-issue
24+
# impact:
25+
26+
# REQUIRED for breaking-change, deprecation, known-issue
27+
# action:
28+
29+
# REQUIRED for all kinds
30+
# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
31+
component: packetbeat
32+
33+
# AUTOMATED
34+
# OPTIONAL to manually add other PR URLs
35+
# PR URL: A link the PR that added the changeset.
36+
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
37+
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
38+
# Please provide it if you are adding a fragment for a different PR.
39+
# pr: https://github.com/owner/repo/1234
40+
41+
# AUTOMATED
42+
# OPTIONAL to manually add other issue URLs
43+
# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
44+
# If not present is automatically filled by the tooling with the issue linked to the PR number.
45+
# issue: https://github.com/owner/repo/1234

‎packetbeat/protos/pgsql/parse.go‎

Lines changed: 78 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package pgsql
1919

2020
import (
2121
"errors"
22+
"fmt"
2223
"strings"
2324

2425
"github.com/elastic/beats/v7/libbeat/common"
@@ -85,7 +86,12 @@ func (pgsql *pgsqlPlugin) parseMessageStart(s *pgsqlStream) (bool, bool) {
8586
s.parseOffset += length
8687
m.end = s.parseOffset
8788
m.isSSLRequest = true
88-
m.size = uint64(m.end - m.start)
89+
msgSize := m.end - m.start
90+
if msgSize < 0 {
91+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
92+
return false, false
93+
}
94+
m.size = uint64(msgSize)
8995

9096
return true, true
9197
}
@@ -94,7 +100,7 @@ func (pgsql *pgsqlPlugin) parseMessageStart(s *pgsqlStream) (bool, bool) {
94100

95101
func (pgsql *pgsqlPlugin) parseCommand(s *pgsqlStream) (bool, bool) {
96102
// read type
97-
typ := byte(s.data[s.parseOffset])
103+
typ := s.data[s.parseOffset]
98104

99105
if s.expectSSLResponse {
100106
// SSLRequest was received in the other stream
@@ -107,7 +113,12 @@ func (pgsql *pgsqlPlugin) parseCommand(s *pgsqlStream) (bool, bool) {
107113
s.parseOffset++
108114
m.end = s.parseOffset
109115
m.isSSLResponse = true
110-
m.size = uint64(m.end - m.start)
116+
msgSize := m.end - m.start
117+
if msgSize < 0 {
118+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
119+
return false, false
120+
}
121+
m.size = uint64(msgSize)
111122

112123
return true, true
113124
}
@@ -161,7 +172,12 @@ func (pgsql *pgsqlPlugin) parseSimpleQuery(s *pgsqlStream, length int) (bool, bo
161172
s.parseOffset++ // type
162173
s.parseOffset += length
163174
m.end = s.parseOffset
164-
m.size = uint64(m.end - m.start)
175+
msgSize := m.end - m.start
176+
if msgSize < 0 {
177+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
178+
return false, false
179+
}
180+
m.size = uint64(msgSize)
165181

166182
query, err := pgsqlString(s.data[m.start+5:], length-4)
167183
if err != nil {
@@ -224,7 +240,12 @@ func (pgsql *pgsqlPlugin) parseEmptyQueryResponse(s *pgsqlStream) (bool, bool) {
224240
m.toExport = true
225241
s.parseOffset += 5 // type + length
226242
m.end = s.parseOffset
227-
m.size = uint64(m.end - m.start)
243+
msgSize := m.end - m.start
244+
if msgSize < 0 {
245+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
246+
return false, false
247+
}
248+
m.size = uint64(msgSize)
228249

229250
return true, true
230251
}
@@ -248,7 +269,12 @@ func (pgsql *pgsqlPlugin) parseCommandComplete(s *pgsqlStream, length int) (bool
248269

249270
s.parseOffset += length
250271
m.end = s.parseOffset
251-
m.size = uint64(m.end - m.start)
272+
msgSize := m.end - m.start
273+
if msgSize < 0 {
274+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
275+
return false, false
276+
}
277+
m.size = uint64(msgSize)
252278

253279
return true, true
254280
}
@@ -257,7 +283,12 @@ func (pgsql *pgsqlPlugin) parseReadyForQuery(s *pgsqlStream, length int) (bool,
257283
// ReadyForQuery -> backend ready for a new query cycle
258284
m := s.message
259285
m.start = s.parseOffset
260-
m.size = uint64(m.end - m.start)
286+
msgSize := m.end - m.start
287+
if msgSize < 0 {
288+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
289+
return false, false
290+
}
291+
m.size = uint64(msgSize)
261292

262293
s.parseOffset++ // type
263294
s.parseOffset += length
@@ -281,7 +312,12 @@ func (pgsql *pgsqlPlugin) parseErrorResponse(s *pgsqlStream, length int) (bool,
281312

282313
s.parseOffset += length // length
283314
m.end = s.parseOffset
284-
m.size = uint64(m.end - m.start)
315+
msgSize := m.end - m.start
316+
if msgSize < 0 {
317+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
318+
return false, false
319+
}
320+
m.size = uint64(msgSize)
285321

286322
return true, true
287323
}
@@ -297,7 +333,12 @@ func (pgsql *pgsqlPlugin) parseExtReq(s *pgsqlStream, length int) (bool, bool) {
297333
s.parseOffset++ // type
298334
s.parseOffset += length
299335
m.end = s.parseOffset
300-
m.size = uint64(m.end - m.start)
336+
msgSize := m.end - m.start
337+
if msgSize < 0 {
338+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
339+
return false, false
340+
}
341+
m.size = uint64(msgSize)
301342
m.toExport = true
302343

303344
query, err := common.ReadString(s.data[m.start+6:])
@@ -341,7 +382,12 @@ func (pgsql *pgsqlPlugin) parseSkipMessage(s *pgsqlStream, length int) (bool, bo
341382

342383
m := s.message
343384
m.end = s.parseOffset
344-
m.size = uint64(m.end - m.start)
385+
msgSize := m.end - m.start
386+
if msgSize < 0 {
387+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
388+
return false, false
389+
}
390+
m.size = uint64(msgSize)
345391

346392
// ok and complete, but ignore
347393
m.toExport = false
@@ -456,7 +502,7 @@ func (pgsql *pgsqlPlugin) parseMessageData(s *pgsqlStream) (bool, bool) {
456502

457503
for len(s.data[s.parseOffset:]) > 5 {
458504
// read type
459-
typ := byte(s.data[s.parseOffset])
505+
typ := s.data[s.parseOffset]
460506

461507
// read message length
462508
length := readLength(s.data[s.parseOffset+1:])
@@ -475,6 +521,7 @@ func (pgsql *pgsqlPlugin) parseMessageData(s *pgsqlStream) (bool, bool) {
475521
case 'D':
476522
err := pgsql.parseDataRow(s, s.data[s.parseOffset+5:s.parseOffset+length+1])
477523
if err != nil {
524+
pgsql.detailf("error parsing data row: %s", err)
478525
return false, false
479526
}
480527
s.parseOffset++
@@ -494,7 +541,12 @@ func (pgsql *pgsqlPlugin) parseMessageData(s *pgsqlStream) (bool, bool) {
494541
pgsql.detailf("CommandComplete length=%d, tag=%s", length, name)
495542
s.parseOffset += length
496543
m.end = s.parseOffset
497-
m.size = uint64(m.end - m.start)
544+
msgSize := m.end - m.start
545+
if msgSize < 0 {
546+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
547+
return false, false
548+
}
549+
m.size = uint64(msgSize)
498550
s.parseState = pgsqlStartState
499551

500552
pgsql.detailf("Rows: %s", m.rows)
@@ -531,7 +583,10 @@ func (pgsql *pgsqlPlugin) parseDataRow(s *pgsqlStream, buf []byte) error {
531583
rows := []string{}
532584
rowLength := 0
533585

534-
for i := 0; i < fieldCount; i++ {
586+
if fieldCount > len(m.fieldsFormat) {
587+
return fmt.Errorf("%w: DataRow field mismatch, got %d, expected %d", errFieldBufferBig, fieldCount, len(m.fieldsFormat))
588+
}
589+
for field := range fieldCount {
535590
if len(buf) <= off {
536591
return errFieldBufferShort
537592
}
@@ -541,14 +596,14 @@ func (pgsql *pgsqlPlugin) parseDataRow(s *pgsqlStream, buf []byte) error {
541596
off += 4
542597

543598
if columnLength > 0 && columnLength > len(buf[off:]) {
544-
pgsql.log.Errorf("Pgsql invalid column_length=%v, buffer_length=%v, i=%v",
545-
columnLength, len(buf[off:]), i)
599+
pgsql.log.Errorf("Pgsql invalid column_length=%v, buffer_length=%v, field=%v",
600+
columnLength, len(buf[off:]), field)
546601
return errInvalidLength
547602
}
548603

549604
// read column value (byten)
550605
var columnValue []byte
551-
if m.fieldsFormat[i] == 0 {
606+
if m.fieldsFormat[field] == 0 {
552607
// field value in text format
553608
if columnLength > 0 {
554609
columnValue = buf[off : off+columnLength]
@@ -593,7 +648,7 @@ func (pgsql *pgsqlPlugin) parseMessageExtendedQuery(s *pgsqlStream) (bool, bool)
593648

594649
for len(s.data[s.parseOffset:]) >= 5 {
595650
// read type
596-
typ := byte(s.data[s.parseOffset])
651+
typ := s.data[s.parseOffset]
597652

598653
// read message length
599654
length := readLength(s.data[s.parseOffset+1:])
@@ -637,7 +692,12 @@ func (pgsql *pgsqlPlugin) parseMessageExtendedQuery(s *pgsqlStream) (bool, bool)
637692
s.parseOffset++
638693
s.parseOffset += length
639694
m.end = s.parseOffset
640-
m.size = uint64(m.end - m.start)
695+
msgSize := m.end - m.start
696+
if msgSize < 0 {
697+
pgsql.detailf("invalid size: start: %d end: %d", m.start, m.end)
698+
return false, false
699+
}
700+
m.size = uint64(msgSize)
641701
s.parseState = pgsqlStartState
642702

643703
return true, true

‎packetbeat/protos/pgsql/pgsql_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,21 @@ func TestPgsqlParser_response(t *testing.T) {
182182
}
183183
}
184184

185+
// Test a DataRow that that reports as having only one row, but actually has two.
186+
func TestPgslMaliciousDataRowLen(t *testing.T) {
187+
data := "540000001a00016100000000000001000000170004ffffffff0000" + "4400000010000200000001580000000159"
188+
pgsql := pgsqlModForTests(nil)
189+
message, err := hex.DecodeString(string(data))
190+
assert.NoError(t, err)
191+
192+
stream := &pgsqlStream{data: message, message: new(pgsqlMessage)}
193+
194+
// make sure we don't panic
195+
ok, complete := pgsql.pgsqlMessageParser(stream)
196+
assert.False(t, ok)
197+
assert.False(t, complete)
198+
}
199+
185200
// Test parsing an incomplete pgsql response
186201
func TestPgsqlParser_incomplete_response(t *testing.T) {
187202
logp.TestingSetup(logp.WithSelectors("pgsql", "pgsqldetailed"))

0 commit comments

Comments
 (0)