Repository: quic-go/quic-go
Pull request #4180 (2023-11-23 07:16:23)

wire: reject NEW_CONNECTION_ID frames with zero-length conneciton IDs

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4180?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`3bf2e19`)](https://app.codecov.io/gh/quic-go/quic-go/commit/3bf2e19d0dc617135ec9d6f3c5191740a27097c7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) 83.77% compared to head [(`9d62d9d`)](https://app.codecov.io/gh/quic-go/quic-go/pull/4180?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) 83.80%.
> Report is 1 commits behind head on master.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4180      +/-   ##
==========================================
+ Coverage   83.77%   83.80%   +0.02%     
==========================================
  Files         150      150              
  Lines       15425    15428       +3     
==========================================
+ Hits        12922    12928       +6     
+ Misses       2006     2003       -3     
  Partials      497      497              
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4180?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).


Patch:
diff --git a/internal/wire/new_connection_id_frame.go b/internal/wire/new_connection_id_frame.go
index 83102d5d..713ab996 100644
--- a/internal/wire/new_connection_id_frame.go
+++ b/internal/wire/new_connection_id_frame.go
@@ -2,6 +2,7 @@ package wire
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 
@@ -34,6 +35,9 @@ func parseNewConnectionIDFrame(r *bytes.Reader, _ protocol.VersionNumber) (*NewC
 	if err != nil {
 		return nil, err
 	}
+	if connIDLen == 0 {
+		return nil, errors.New("invalid zero-length connection ID")
+	}
 	connID, err := protocol.ReadConnectionID(r, int(connIDLen))
 	if err != nil {
 		return nil, err


Test patch:
diff --git a/internal/wire/new_connection_id_frame_test.go b/internal/wire/new_connection_id_frame_test.go
index 4628aaf2..02ef3000 100644
--- a/internal/wire/new_connection_id_frame_test.go
+++ b/internal/wire/new_connection_id_frame_test.go
@@ -38,7 +38,15 @@ var _ = Describe("NEW_CONNECTION_ID frame", func() {
 			Expect(err).To(MatchError("Retire Prior To value (1001) larger than Sequence Number (1000)"))
 		})
 
-		It("errors when the connection ID has an invalid length", func() {
+		It("errors when the connection ID has a zero-length connection ID", func() {
+			data := encodeVarInt(42)                 // sequence number
+			data = append(data, encodeVarInt(12)...) // retire prior to
+			data = append(data, 0)                   // connection ID length
+			_, err := parseNewConnectionIDFrame(bytes.NewReader(data), protocol.Version1)
+			Expect(err).To(MatchError("invalid zero-length connection ID"))
+		})
+
+		It("errors when the connection ID has an invalid length (too long)", func() {
 			data := encodeVarInt(0xdeadbeef)                                                                          // sequence number
 			data = append(data, encodeVarInt(0xcafe)...)                                                              // retire prior to
 			data = append(data, 21)                                                                                   // connection ID length