amacneil / amacneil/dbmate

TrimLeadingSQLComments reads past line bounds on single-character lines (potential panic / wrong skip)

未關閉 適合新手
#828 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
主要語言
Go
星號
7.4k
分支
380
平均合併
1 天 16 小時
30 天內合併 PR
2

描述

## Description

`dbutil.TrimLeadingSQLComments` indexes `line[0:2]` without checking that the line has at least two bytes. For a one-character line inside the leading-comment block, this either panics or silently compares bytes beyond the end of the current line, so the function can crash or wrongly drop a data line from a schema dump.

*This report is based on static analysis of the source (master); I have not executed the code.*

- Version: master (`pkg/dbutil/dbutil.go`)
- Database: affects all drivers using this helper for dump post-processing (postgres, mysql, sqlite)
- Operating System: any

## Location

- File: `pkg/dbutil/dbutil.go`
- Function: `TrimLeadingSQLComments`
- Relevant code path:

```go
scanner := bufio.NewScanner(bytes.NewReader(data))
for scanner.Scan() {
line := scanner.Bytes()

if preamble && (len(line) == 0 || bytes.Equal(line[0:2], []byte("--"))) {
continue
}
...
```

## Problem

The preamble check tests `len(line) == 0 || bytes.Equal(line[0:2], []byte("--"))`. When `len(line) == 1`, the expression `line[0:2]` slices past the end of the token:

1. **Panic case:** Go slicing requires `high <= cap(s)`. If the underlying array shared by `scanner.Bytes()` does not have two addressable bytes after the token start, `line[0:2]` panics with *slice bounds out of range*. `bufio.Scanner.Bytes()` returns a view into the scanner's internal buffer, whose remaining capacity at that offset depends on buffer growth/sliding — so whether this panics depends on where the line sits in the buffer, not just on its length.
2. **Wrong-comparison case:** when capacity happens to be sufficient, `bytes.Equal(line[0:2], ...)` still reads a byte that is not part of the current line (the following newline, or stale bytes from a previous read chunk after the scanner reuses its buffer). The comparison result is then determined by unrelated buffer content: if those bytes happen to be `"--"`, a legitimate one-character data line is silently skipped and removed from the dump; otherwise it is kept.

## Steps To Reproduce

Conceptually, call:

```go
dbutil.TrimLeadingSQLComments([]byte("-- header\n-\nSELECT 1;\n"))
```

The second line (`-`, length 1) reaches the `preamble` branch with `len(line) == 1`, forcing evaluation of `line[0:2]`. Whether this panics or miscompares depends on the scanner's internal buffer state at that offset (see above). A unit test with inputs engineered around the ~4KB scanner-buffer boundary should expose both modes.

## Expected Behavior

A one-character line should simply fail the "is it a comment?" test (it cannot start with `--`) and be preserved in the output, without any out-of-bounds access.

## Actual Behavior

`line[0:2]` performs an out-of-bounds-capacity slice for length-1 lines: potential panic, and an unreliable comparison that can drop lines from schema dumps processed by `DumpSchema` implementations.

## Impact

`TrimLeadingSQLComments` post-processes `pg_dump`/`mysqldump` output in the postgres, mysql and sqlite drivers. A crash aborts dumping; the wrong-skip mode would corrupt `schema.sql` by deleting a real line from the dumped SQL. Both failure modes are silent or confusing for users.

## Suggested Direction

Guard the slice, e.g. `len(line) >= 2 && bytes.Equal(line[0:2], []byte("--"))` (the zero-length case is already handled), and add table-driven cases with single-character lines inside and after the preamble.

貢獻指南

這個儲存庫沒有索引到貢獻指南

研究方向

Start in pkg/dbutil/dbutil.go at TrimLeadingSQLComments and inspect how bufio.Scanner processes each line. Add table-driven coverage for single-character lines inside and after the leading-comment block, including the provided dump-shaped input, then run the relevant Go package tests and confirm those lines are preserved without a panic.

由索引模型根據 Issue 內容生成。

評估

技術堆疊
go, mysql, postgresql, sqlite
領域
databases
Issue 類型
缺陷
難度
2/5
預估耗時
1-3 小時
活躍度
活躍
描述清晰度
描述清楚
新手友好度
88/100

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。