Skip to content

Conversation

@fengweiyuan
Copy link

… the EncodeUDPPacket function of proxy/shadowsocks/protocol.go, a fixed-size buffer of 2048 bytes is created using buf.New().

However, the actual size of a UDP packet includes: IV: 16 bytes (e.g., for aes-128-gcm)
Destination address: 7~258 bytes (depending on IPv4/IPv6/domain) Payload: variable, up to 2048 bytes
AEAD authentication tag: 16 bytes
When the incoming payload is large (close to 2048 bytes), the total size exceeds the buffer capacity. When b.Extend() is called in AEADCipher.EncodePacket() to allocate space for the authentication tag, it triggers panic: extending out of bound, causing the process to crash. Fix
Before creating the buffer, calculate the required buffer size based on IV length, maximum address length (258 bytes), payload length, and AEAD overhead (16 bytes). If the required size exceeds the default buffer size (2048 bytes), use buf.NewWithSize() to allocate a sufficiently large buffer.

问题描述
Shadowsocks UDP 数据包编码时存在缓冲区溢出的 bug。在 proxy/shadowsocks/protocol.go 的 EncodeUDPPacket 函数中,使用 buf.New() 创建了一个固定大小为 2048 字节的缓冲区。
然而,UDP 数据包的实际大小需要包含:
IV:16 字节(以 aes-128-gcm 为例)
目标地址:7~258 字节(取决于 IPv4/IPv6/域名)
Payload:可变,最大可达 2048 字节
AEAD 认证标签:16 字节
当传入的 payload 较大(接近 2048 字节)时,总大小会超过缓冲区容量。在调用 AEADCipher.EncodePacket() 中的 b.Extend() 扩展认证标签空间时,会触发 panic: extending out of bound,导致进程崩溃。
修复方案
在创建缓冲区之前,根据 IV 长度、最大地址长度(258 字节)、payload 长度和 AEAD overhead(16 字节)计算所需的缓冲区大小。如果需要的大小超过默认缓冲区大小(2048 字节),则使用 buf.NewWithSize() 分配足够大的缓冲区。

… the EncodeUDPPacket function of proxy/shadowsocks/protocol.go, a fixed-size buffer of 2048 bytes is created using buf.New().

However, the actual size of a UDP packet includes:
IV: 16 bytes (e.g., for aes-128-gcm)
Destination address: 7~258 bytes (depending on IPv4/IPv6/domain)
Payload: variable, up to 2048 bytes
AEAD authentication tag: 16 bytes
When the incoming payload is large (close to 2048 bytes), the total size exceeds the buffer capacity. When b.Extend() is called in AEADCipher.EncodePacket() to allocate space for the authentication tag, it triggers panic: extending out of bound, causing the process to crash.
Fix
Before creating the buffer, calculate the required buffer size based on IV length, maximum address length (258 bytes), payload length, and AEAD overhead (16 bytes). If the required size exceeds the default buffer size (2048 bytes), use buf.NewWithSize() to allocate a sufficiently large buffer.
Copy link
Contributor

@xiaokangwang xiaokangwang left a comment

Choose a reason for hiding this comment

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

Thanks for your work, this looks good to me!

@codecov
Copy link

codecov bot commented Dec 29, 2025

Codecov Report

❌ Patch coverage is 37.50000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 23.22%. Comparing base (7227b98) to head (d1869da).
⚠️ Report is 31 commits behind head on master.

Files with missing lines Patch % Lines
proxy/shadowsocks/protocol.go 37.50% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3564      +/-   ##
==========================================
+ Coverage   22.78%   23.22%   +0.43%     
==========================================
  Files         819      836      +17     
  Lines       52429    45434    -6995     
==========================================
- Hits        11946    10551    -1395     
+ Misses      39279    33668    -5611     
- Partials     1204     1215      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xiaokangwang xiaokangwang merged commit 876d7f7 into v2fly:master Dec 29, 2025
43 checks passed
@dyhkwong
Copy link
Contributor

dyhkwong commented Jan 4, 2026

Why is "max address length" 258? Isn't it 1 byte address type + 1 byte domain length + 255 bytes domain + 2 bytes port, which is equal to 259?
Also, why not calculate the actual length of the address like

var addrPortLen int32
switch dest.Address.Family() {
case net.AddressFamilyDomain:
if protocol.IsDomainTooLong(dest.Address.Domain()) {
return 0, newError("Super long domain is not supported: ", dest.Address.Domain())
}
addrPortLen = 1 + 1 + int32(len(dest.Address.Domain())) + 2
case net.AddressFamilyIPv4:
addrPortLen = 1 + 4 + 2
case net.AddressFamilyIPv6:
addrPortLen = 1 + 16 + 2
default:
panic("Unknown address type.")
}
?

@xiaokangwang
Copy link
Contributor

Why is "max address length" 258? Isn't it 1 byte address type + 1 byte domain length + 255 bytes domain + 2 bytes port, which is equal to 259? Also, why not calculate the actual length of the address like

var addrPortLen int32
switch dest.Address.Family() {
case net.AddressFamilyDomain:
if protocol.IsDomainTooLong(dest.Address.Domain()) {
return 0, newError("Super long domain is not supported: ", dest.Address.Domain())
}
addrPortLen = 1 + 1 + int32(len(dest.Address.Domain())) + 2
case net.AddressFamilyIPv4:
addrPortLen = 1 + 4 + 2
case net.AddressFamilyIPv6:
addrPortLen = 1 + 16 + 2
default:
panic("Unknown address type.")
}

?

I will send a pull request with the suggested change. Sorry for not catching this during review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants