Skip to content

Commit bb30136

Browse files
alexluongclaude
andcommitted
fix: use parameterized queries for cursor-based pagination
Convert buildEventCursorCondition and buildCursorCondition to use parameterized queries with ? placeholders instead of string interpolation to prevent SQL injection vulnerabilities. Add parseTimestampMs helper to validate timestamp strings before use. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 6213d74 commit bb30136

File tree

1 file changed

+67
-35
lines changed

1 file changed

+67
-35
lines changed

internal/logstore/chlogstore/chlogstore.go

Lines changed: 67 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"strconv"
78
"strings"
89
"time"
910

@@ -94,11 +95,13 @@ func (s *logStoreImpl) ListEvent(ctx context.Context, req driver.ListEventReques
9495

9596
// Cursor conditions
9697
if !nextCursor.IsEmpty() {
97-
cursorCond := buildEventCursorCondition(sortOrder, nextCursor.Position, false)
98+
cursorCond, cursorArgs := buildEventCursorCondition(sortOrder, nextCursor.Position, false)
9899
conditions = append(conditions, cursorCond)
100+
args = append(args, cursorArgs...)
99101
} else if !prevCursor.IsEmpty() {
100-
cursorCond := buildEventCursorCondition(sortOrder, prevCursor.Position, true)
102+
cursorCond, cursorArgs := buildEventCursorCondition(sortOrder, prevCursor.Position, true)
101103
conditions = append(conditions, cursorCond)
104+
args = append(args, cursorArgs...)
102105
}
103106

104107
whereClause := strings.Join(conditions, " AND ")
@@ -256,14 +259,18 @@ func (s *logStoreImpl) ListEvent(ctx context.Context, req driver.ListEventReques
256259
}, nil
257260
}
258261

259-
// buildEventCursorCondition builds a SQL condition for cursor-based pagination on events table
260-
func buildEventCursorCondition(sortOrder, position string, isBackward bool) string {
262+
// buildEventCursorCondition builds a SQL condition for cursor-based pagination on events table.
263+
// Returns the condition string with ? placeholders and the corresponding args.
264+
func buildEventCursorCondition(sortOrder, position string, isBackward bool) (string, []interface{}) {
261265
// Parse position: eventTimeMs::eventID
262266
parts := strings.SplitN(position, "::", 2)
263267
if len(parts) != 2 {
264-
return "1=1" // invalid cursor, return always true
268+
return "1=1", nil // invalid cursor, return always true
269+
}
270+
eventTimeMs, err := parseTimestampMs(parts[0])
271+
if err != nil {
272+
return "1=1", nil // invalid timestamp, return always true
265273
}
266-
eventTimeMs := parts[0]
267274
eventID := parts[1]
268275

269276
// Determine comparison direction
@@ -283,10 +290,13 @@ func buildEventCursorCondition(sortOrder, position string, isBackward bool) stri
283290
}
284291

285292
// Build multi-column comparison for (event_time, event_id)
286-
return fmt.Sprintf(`(
287-
event_time %s fromUnixTimestamp64Milli(%s)
288-
OR (event_time = fromUnixTimestamp64Milli(%s) AND event_id %s '%s')
289-
)`, cmp, eventTimeMs, eventTimeMs, cmp, eventID)
293+
// Uses parameterized queries to prevent SQL injection
294+
condition := fmt.Sprintf(`(
295+
event_time %s fromUnixTimestamp64Milli(?)
296+
OR (event_time = fromUnixTimestamp64Milli(?) AND event_id %s ?)
297+
)`, cmp, cmp)
298+
299+
return condition, []interface{}{eventTimeMs, eventTimeMs, eventID}
290300
}
291301

292302
func (s *logStoreImpl) ListDeliveryEvent(ctx context.Context, req driver.ListDeliveryEventRequest) (driver.ListDeliveryEventResponse, error) {
@@ -399,11 +409,13 @@ func (s *logStoreImpl) ListDeliveryEvent(ctx context.Context, req driver.ListDel
399409

400410
// Cursor conditions
401411
if !nextCursor.IsEmpty() {
402-
cursorCond := buildCursorCondition(sortBy, sortOrder, nextCursor.Position, false)
412+
cursorCond, cursorArgs := buildCursorCondition(sortBy, sortOrder, nextCursor.Position, false)
403413
conditions = append(conditions, cursorCond)
414+
args = append(args, cursorArgs...)
404415
} else if !prevCursor.IsEmpty() {
405-
cursorCond := buildCursorCondition(sortBy, sortOrder, prevCursor.Position, true)
416+
cursorCond, cursorArgs := buildCursorCondition(sortBy, sortOrder, prevCursor.Position, true)
406417
conditions = append(conditions, cursorCond)
418+
args = append(args, cursorArgs...)
407419
}
408420

409421
whereClause := strings.Join(conditions, " AND ")
@@ -925,8 +937,14 @@ func (s *logStoreImpl) InsertManyDeliveryEvent(ctx context.Context, deliveryEven
925937
return nil
926938
}
927939

928-
// buildCursorCondition builds a SQL condition for cursor-based pagination
929-
func buildCursorCondition(sortBy, sortOrder, position string, isBackward bool) string {
940+
// parseTimestampMs parses a millisecond timestamp string to int64.
941+
func parseTimestampMs(s string) (int64, error) {
942+
return strconv.ParseInt(s, 10, 64)
943+
}
944+
945+
// buildCursorCondition builds a SQL condition for cursor-based pagination.
946+
// Returns the condition string with ? placeholders and the corresponding args.
947+
func buildCursorCondition(sortBy, sortOrder, position string, isBackward bool) (string, []interface{}) {
930948
// Parse position based on sortBy
931949
// For delivery_time: "timestamp::deliveryID"
932950
// For event_time: "timestamp::eventID::timestamp::deliveryID"
@@ -935,11 +953,17 @@ func buildCursorCondition(sortBy, sortOrder, position string, isBackward bool) s
935953
// Parse: eventTimeMs::eventID::deliveryTimeMs::deliveryID
936954
parts := strings.SplitN(position, "::", 4)
937955
if len(parts) != 4 {
938-
return "1=1" // invalid cursor, return always true
956+
return "1=1", nil // invalid cursor, return always true
957+
}
958+
eventTimeMs, err := parseTimestampMs(parts[0])
959+
if err != nil {
960+
return "1=1", nil // invalid timestamp, return always true
939961
}
940-
eventTimeMs := parts[0]
941962
eventID := parts[1]
942-
deliveryTimeMs := parts[2]
963+
deliveryTimeMs, err := parseTimestampMs(parts[2])
964+
if err != nil {
965+
return "1=1", nil // invalid timestamp, return always true
966+
}
943967
deliveryID := parts[3]
944968

945969
// Determine comparison direction
@@ -958,27 +982,33 @@ func buildCursorCondition(sortBy, sortOrder, position string, isBackward bool) s
958982
}
959983
}
960984

961-
// Build multi-column comparison
985+
// Build multi-column comparison with parameterized queries
962986
// (event_time, event_id, delivery_time, delivery_id) < (cursor_values)
963-
return fmt.Sprintf(`(
964-
event_time %s fromUnixTimestamp64Milli(%s)
965-
OR (event_time = fromUnixTimestamp64Milli(%s) AND event_id %s '%s')
966-
OR (event_time = fromUnixTimestamp64Milli(%s) AND event_id = '%s' AND delivery_time %s fromUnixTimestamp64Milli(%s))
967-
OR (event_time = fromUnixTimestamp64Milli(%s) AND event_id = '%s' AND delivery_time = fromUnixTimestamp64Milli(%s) AND delivery_id %s '%s')
968-
)`,
969-
cmp, eventTimeMs,
970-
eventTimeMs, cmp, eventID,
971-
eventTimeMs, eventID, cmp, deliveryTimeMs,
972-
eventTimeMs, eventID, deliveryTimeMs, cmp, deliveryID,
973-
)
987+
condition := fmt.Sprintf(`(
988+
event_time %s fromUnixTimestamp64Milli(?)
989+
OR (event_time = fromUnixTimestamp64Milli(?) AND event_id %s ?)
990+
OR (event_time = fromUnixTimestamp64Milli(?) AND event_id = ? AND delivery_time %s fromUnixTimestamp64Milli(?))
991+
OR (event_time = fromUnixTimestamp64Milli(?) AND event_id = ? AND delivery_time = fromUnixTimestamp64Milli(?) AND delivery_id %s ?)
992+
)`, cmp, cmp, cmp, cmp)
993+
994+
args := []interface{}{
995+
eventTimeMs,
996+
eventTimeMs, eventID,
997+
eventTimeMs, eventID, deliveryTimeMs,
998+
eventTimeMs, eventID, deliveryTimeMs, deliveryID,
999+
}
1000+
return condition, args
9741001
}
9751002

9761003
// delivery_time sort: "timestamp::deliveryID"
9771004
parts := strings.SplitN(position, "::", 2)
9781005
if len(parts) != 2 {
979-
return "1=1"
1006+
return "1=1", nil
1007+
}
1008+
deliveryTimeMs, err := parseTimestampMs(parts[0])
1009+
if err != nil {
1010+
return "1=1", nil // invalid timestamp, return always true
9801011
}
981-
deliveryTimeMs := parts[0]
9821012
deliveryID := parts[1]
9831013

9841014
var cmp string
@@ -996,8 +1026,10 @@ func buildCursorCondition(sortBy, sortOrder, position string, isBackward bool) s
9961026
}
9971027
}
9981028

999-
return fmt.Sprintf(`(
1000-
delivery_time %s fromUnixTimestamp64Milli(%s)
1001-
OR (delivery_time = fromUnixTimestamp64Milli(%s) AND delivery_id %s '%s')
1002-
)`, cmp, deliveryTimeMs, deliveryTimeMs, cmp, deliveryID)
1029+
condition := fmt.Sprintf(`(
1030+
delivery_time %s fromUnixTimestamp64Milli(?)
1031+
OR (delivery_time = fromUnixTimestamp64Milli(?) AND delivery_id %s ?)
1032+
)`, cmp, cmp)
1033+
1034+
return condition, []interface{}{deliveryTimeMs, deliveryTimeMs, deliveryID}
10031035
}

0 commit comments

Comments
 (0)