[Bug] protocol.go: BytesToInt lacks buffer bounds checking — potential panic on malformed data
Description
The BytesToInt function reads an int32 from a byte slice without verifying the slice has enough data. Malformed network packets with truncated length fields can cause a panic (index out of range) in the distributed communication layer.
Context
- File: app/distribute/teleport/protocol.go:66-72
- Function: BytesToInt
Current vs Expected Behavior
Current:
func BytesToInt(b []byte) int {
bytesBuffer := bytes.NewBuffer(b)
var x int32
binary.Read(bytesBuffer, binary.LittleEndian, &x)
return int(x)
}
This function assumes b has at least 4 bytes. If b is shorter (due to malformed input), binary.Read will fail silently but may not panic immediately. However, subsequent use of the returned length in Unpack can cause index out of range.
Expected:
Validate buffer length before reading.
Suggested Fix
func BytesToInt(b []byte) int {
if len(b) < 4 {
return 0 // Or return an error: return -1, fmt.Errorf("buffer too short")
}
bytesBuffer := bytes.NewBuffer(b)
var x int32
if err := binary.Read(bytesBuffer, binary.LittleEndian, &x); err != nil {
return 0 // Or handle error appropriately
}
return int(x)
}
Additionally, the Unpack function should validate messageLength before allocation:
func (p *Protocol) Unpack(buffer []byte) (readerSlice [][]byte, bufferOver []byte) {
// ... existing code ...
messageLength := BytesToInt(buffer[i+p.headerLen : i+p.headerLen+DataLengthOfLenth])
if messageLength <= 0 || messageLength > MaxMessageLength {
// Skip malformed packet
i += p.headerLen + DataLengthOfLenth
continue
}
// ... rest of code ...
}
Consider adding a MaxMessageLength constant to prevent OOM from giant length fields.
Impact
- Severity: Medium
- Affected: All users in distributed mode
- Attack vector: Malicious or corrupted network packets can crash the master/slave nodes
- Risk: Denial of service via crafted packets
Positively — happy to submit a PR if this is welcome.
[Bug] protocol.go: BytesToInt lacks buffer bounds checking — potential panic on malformed data
Description
The BytesToInt function reads an int32 from a byte slice without verifying the slice has enough data. Malformed network packets with truncated length fields can cause a panic (index out of range) in the distributed communication layer.
Context
Current vs Expected Behavior
Current:
func BytesToInt(b []byte) int {
bytesBuffer := bytes.NewBuffer(b)
}
This function assumes b has at least 4 bytes. If b is shorter (due to malformed input), binary.Read will fail silently but may not panic immediately. However, subsequent use of the returned length in Unpack can cause index out of range.
Expected:
Validate buffer length before reading.
Suggested Fix
func BytesToInt(b []byte) int {
if len(b) < 4 {
return 0 // Or return an error: return -1, fmt.Errorf("buffer too short")
}
bytesBuffer := bytes.NewBuffer(b)
}
Additionally, the Unpack function should validate messageLength before allocation:
func (p *Protocol) Unpack(buffer []byte) (readerSlice [][]byte, bufferOver []byte) {
// ... existing code ...
messageLength := BytesToInt(buffer[i+p.headerLen : i+p.headerLen+DataLengthOfLenth])
if messageLength <= 0 || messageLength > MaxMessageLength {
// Skip malformed packet
i += p.headerLen + DataLengthOfLenth
continue
}
// ... rest of code ...
}
Consider adding a MaxMessageLength constant to prevent OOM from giant length fields.
Impact
Positively — happy to submit a PR if this is welcome.