Skip to content

QUIC sniffer: Fix potential slice panic #3406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xiaokangwang merged 5 commits into from
May 17, 2025
Merged

QUIC sniffer: Fix potential slice panic #3406

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ada3033
QUIC sniffer: Fix potential slice panic
Fangliding May 9, 2025
374176d
Fix type err
Fangliding May 9, 2025
be67b99
Update sniff.go
Fangliding May 9, 2025
d92de2f
Add test and fix more
Fangliding May 9, 2025
de94088
Refine
Fangliding May 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion common/protocol/quic/sniff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func SniffQUIC(b []byte) (*SniffHeader, error) {
}

hdrLen := len(b) - int(buffer.Len())
if len(b) < hdrLen+int(packetLen) {
if len(b) < hdrLen+int(packetLen) || len(b) < hdrLen+20{
return nil, common.ErrNoClue // Not enough data to read as a QUIC packet. QUIC is UDP-based, so this is unlikely to happen.

@Vigilans Vigilans May 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

翻了一下协议好像没找到下面20(4+16)长度的准确解释，貌似是关于Packet Number Decoding的？

这一行可以付上 // See https://github.com/v2fly/v2ray-core/pull/3406#issuecomment-2866415064，方便查阅这个Magic Number的解释。

@Fangliding Fangliding May 10, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

翻了一下协议好像没找到下面20(4+16)长度的准确解释，貌似是关于Packet Number Decoding的？

这一行可以付上 // See https://github.com/v2fly/v2ray-core/pull/3406#issuecomment-2866415064，方便查阅这个Magic Number的解释。

如果地方多的话 感觉可能不如以前的xray偷懒做法 直接recover切片类panic当read错误返回notquic就行了

@dyhkwong dyhkwong May 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这两个用例都是少了 115 行 packetLen < 4（即 origPNBytes 的长度）的判断，关于 packetNumberLength 应该另外处理（如 #3389）

}

Expand Down Expand Up @@ -166,6 +166,10 @@ func SniffQUIC(b []byte) (*SniffHeader, error) {
}

extHdrLen := hdrLen + int(packetNumberLength)
// very strange packet length, maybe a fake QUIC header
if int(packetNumberLength) > int(packetLen) {
return nil, errNotQuic
}
copy(b[extHdrLen:hdrLen+4], origPNBytes[packetNumberLength:])
data := b[extHdrLen : int(packetLen)+hdrLen]

Expand Down
18 changes: 18 additions & 0 deletions common/protocol/quic/sniff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,21 @@ func TestSniffQUICComplex(t *testing.T) {
})
}
}

func TestSniffFakeQUICPacketWithInvalidPacketNumberLength(t *testing.T) {
pkt, err := hex.DecodeString("cb00000001081c8c6d5aeb53d54400000090709b8600000000000000000000000000000000")
common.Must(err)
_, err = quic.SniffQUIC(pkt)
if err == nil {
t.Error("failed")
}
}

func TestSniffFakeQUICPacketWithTooShortData(t *testing.T) {
pkt, err := hex.DecodeString("cb00000001081c8c6d5aeb53d54400000090709b86")
common.Must(err)
_, err = quic.SniffQUIC(pkt)
if err == nil {
t.Error("failed")
}
}