[go] crypto/tls: avoid overflow in parseECHConfigList

1 view
Skip to first unread message

Nicholas Husin (Gerrit)

unread,
Jul 22, 2026, 12:13:44 AM (4 days ago) Jul 22
to goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Neal Patel, Roland Shoemaker, golang-co...@googlegroups.com
Attention needed from Neal Patel, Nicholas Husin and Roland Shoemaker

Message from Nicholas Husin

Set Ready For Review

Open in Gerrit

Related details

Attention is currently required from:
  • Neal Patel
  • Nicholas Husin
  • Roland Shoemaker
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
Gerrit-Change-Number: 804040
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Husin <n...@golang.org>
Gerrit-Reviewer: Neal Patel <ne...@golang.org>
Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
Gerrit-Reviewer: Roland Shoemaker <rol...@golang.org>
Gerrit-Attention: Roland Shoemaker <rol...@golang.org>
Gerrit-Attention: Neal Patel <ne...@golang.org>
Gerrit-Attention: Nicholas Husin <n...@golang.org>
Gerrit-Comment-Date: Wed, 22 Jul 2026 04:13:39 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
unsatisfied_requirement
satisfied_requirement
open
diffy

Daniel McCarney (Gerrit)

unread,
Jul 22, 2026, 10:35:06 AM (4 days ago) Jul 22
to Nicholas Husin, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Neal Patel, Roland Shoemaker, golang-co...@googlegroups.com
Attention needed from Neal Patel, Nicholas Husin and Roland Shoemaker

Daniel McCarney voted and added 2 comments

Votes added by Daniel McCarney

Code-Review+2

2 comments

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Daniel McCarney . resolved

The fix & test look good to me, but I think the malicious server angle referenced by the commit message could use a bit more supporting analysis.

Commit Message
Line 13, Patchset 1 (Latest):hang.
Daniel McCarney . unresolved

Can you expand on this point? I'm not sure I see a way that a malicious server could deliver a triggering ECH config list payload. I agree it's worth fixing in either case, but I think the impact might be overstated (or I'm missing a detail).

AFAICT, there are ~2 primary methods an ECH config list is likely to arrive. 1. by way of a TLS hello retry request (HRR) providing retry configs 2. By DNS HTTPS-record SvcParams.

The first would be bounded by a uint16 length prefixed handshake message (`opaque<0..2^16-1>`), and the second would be bounded by "a 2-octet field containing the length of the SvcParamValue as an integer between 0 and 65535 in network byte order." (rfc9460)

A 65532 byte long ECHConfig would be 65536 bytes on the wire, plus another 2 for the header, like the 65538 payload from TestDecodeECHConfigListOverflow. Unless I'm mistaken I think that isn't something that could be delivered by a malicious server in practice.

Open in Gerrit

Related details

Attention is currently required from:
  • Neal Patel
  • Nicholas Husin
  • Roland Shoemaker
Submit Requirements:
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
Gerrit-Change-Number: 804040
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Husin <n...@golang.org>
Gerrit-Reviewer: Daniel McCarney <dan...@binaryparadox.net>
Gerrit-Reviewer: Neal Patel <ne...@golang.org>
Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
Gerrit-Reviewer: Roland Shoemaker <rol...@golang.org>
Gerrit-Attention: Roland Shoemaker <rol...@golang.org>
Gerrit-Attention: Neal Patel <ne...@golang.org>
Gerrit-Attention: Nicholas Husin <n...@golang.org>
Gerrit-Comment-Date: Wed, 22 Jul 2026 14:34:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Nicholas Husin (Gerrit)

unread,
Jul 22, 2026, 12:48:06 PM (4 days ago) Jul 22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Neal Patel, Nicholas Husin and Roland Shoemaker

Nicholas Husin uploaded new patchset

Nicholas Husin uploaded patch set #2 to this change.
Open in Gerrit

Related details

Attention is currently required from:
  • Neal Patel
  • Nicholas Husin
  • Roland Shoemaker
Submit Requirements:
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newpatchset
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
Gerrit-Change-Number: 804040
Gerrit-PatchSet: 2
satisfied_requirement
unsatisfied_requirement
open
diffy

Nicholas Husin (Gerrit)

unread,
Jul 22, 2026, 12:48:45 PM (4 days ago) Jul 22
to goph...@pubsubhelper.golang.org, Daniel McCarney, golang...@luci-project-accounts.iam.gserviceaccount.com, Neal Patel, Roland Shoemaker, golang-co...@googlegroups.com
Attention needed from Neal Patel and Roland Shoemaker

Nicholas Husin added 1 comment

Commit Message
Line 13, Patchset 1:hang.
Daniel McCarney . resolved

Can you expand on this point? I'm not sure I see a way that a malicious server could deliver a triggering ECH config list payload. I agree it's worth fixing in either case, but I think the impact might be overstated (or I'm missing a detail).

AFAICT, there are ~2 primary methods an ECH config list is likely to arrive. 1. by way of a TLS hello retry request (HRR) providing retry configs 2. By DNS HTTPS-record SvcParams.

The first would be bounded by a uint16 length prefixed handshake message (`opaque<0..2^16-1>`), and the second would be bounded by "a 2-octet field containing the length of the SvcParamValue as an integer between 0 and 65535 in network byte order." (rfc9460)

A 65532 byte long ECHConfig would be 65536 bytes on the wire, plus another 2 for the header, like the 65538 payload from TestDecodeECHConfigListOverflow. Unless I'm mistaken I think that isn't something that could be delivered by a malicious server in practice.

Nicholas Husin

Ah, indeed, thanks for pointing that out! Updated the commit message and the issue.

Open in Gerrit

Related details

Attention is currently required from:
  • Neal Patel
  • Roland Shoemaker
Submit Requirements:
  • requirement satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
Gerrit-Change-Number: 804040
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Husin <n...@golang.org>
Gerrit-Reviewer: Daniel McCarney <dan...@binaryparadox.net>
Gerrit-Reviewer: Neal Patel <ne...@golang.org>
Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
Gerrit-Reviewer: Roland Shoemaker <rol...@golang.org>
Gerrit-Attention: Roland Shoemaker <rol...@golang.org>
Gerrit-Attention: Neal Patel <ne...@golang.org>
Gerrit-Comment-Date: Wed, 22 Jul 2026 16:48:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Daniel McCarney <dan...@binaryparadox.net>
satisfied_requirement
unsatisfied_requirement
open
diffy

Nicholas Husin (Gerrit)

unread,
Jul 22, 2026, 12:52:49 PM (4 days ago) Jul 22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Neal Patel and Roland Shoemaker

Nicholas Husin uploaded new patchset

Nicholas Husin uploaded patch set #3 to this change.
Open in Gerrit

Related details

Attention is currently required from:
  • Neal Patel
  • Roland Shoemaker
Submit Requirements:
  • requirement satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newpatchset
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
Gerrit-Change-Number: 804040
Gerrit-PatchSet: 3
satisfied_requirement
unsatisfied_requirement
open
diffy

Nicholas Husin (Gerrit)

unread,
Jul 22, 2026, 12:55:34 PM (4 days ago) Jul 22
to goph...@pubsubhelper.golang.org, Daniel McCarney, golang...@luci-project-accounts.iam.gserviceaccount.com, Neal Patel, Roland Shoemaker, golang-co...@googlegroups.com
Attention needed from Neal Patel, Nicholas Husin and Roland Shoemaker

Nicholas Husin added 1 comment

Patchset-level comments
Daniel McCarney . resolved

The fix & test look good to me, but I think the malicious server angle referenced by the commit message could use a bit more supporting analysis.

Nicholas Husin

Thanks for the review! After some discussion with the team, given that the overflow cannot really happen in realistic scenario, we're going to treat this as a security hardening rather than a vulnerability.

Open in Gerrit

Related details

Attention is currently required from:
  • Neal Patel
  • Nicholas Husin
  • Roland Shoemaker
Submit Requirements:
    • requirement satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
    Gerrit-Change-Number: 804040
    Gerrit-PatchSet: 3
    Gerrit-Owner: Nicholas Husin <n...@golang.org>
    Gerrit-Reviewer: Daniel McCarney <dan...@binaryparadox.net>
    Gerrit-Reviewer: Neal Patel <ne...@golang.org>
    Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
    Gerrit-Reviewer: Roland Shoemaker <rol...@golang.org>
    Gerrit-Attention: Roland Shoemaker <rol...@golang.org>
    Gerrit-Attention: Neal Patel <ne...@golang.org>
    Gerrit-Attention: Nicholas Husin <n...@golang.org>
    Gerrit-Comment-Date: Wed, 22 Jul 2026 16:55:26 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Daniel McCarney (Gerrit)

    unread,
    Jul 22, 2026, 12:58:02 PM (4 days ago) Jul 22
    to Nicholas Husin, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Neal Patel, Roland Shoemaker, golang-co...@googlegroups.com
    Attention needed from Neal Patel, Nicholas Husin and Roland Shoemaker

    Daniel McCarney voted and added 1 comment

    Votes added by Daniel McCarney

    Code-Review+2

    1 comment

    Patchset-level comments
    Daniel McCarney . resolved

    The fix & test look good to me, but I think the malicious server angle referenced by the commit message could use a bit more supporting analysis.

    Nicholas Husin

    Thanks for the review! After some discussion with the team, given that the overflow cannot really happen in realistic scenario, we're going to treat this as a security hardening rather than a vulnerability.

    Daniel McCarney

    SGTM! Thanks for fixing.

    Gerrit-Comment-Date: Wed, 22 Jul 2026 16:57:55 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Nicholas Husin <n...@golang.org>
    Comment-In-Reply-To: Daniel McCarney <dan...@binaryparadox.net>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Nicholas Husin (Gerrit)

    unread,
    Jul 22, 2026, 1:49:33 PM (4 days ago) Jul 22
    to Nicholas Husin, goph...@pubsubhelper.golang.org, Daniel McCarney, golang...@luci-project-accounts.iam.gserviceaccount.com, Neal Patel, Roland Shoemaker, golang-co...@googlegroups.com
    Attention needed from Neal Patel, Nicholas Husin and Roland Shoemaker

    Nicholas Husin voted Code-Review+1

    Code-Review+1
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Neal Patel
    • Nicholas Husin
    • Roland Shoemaker
    Submit Requirements:
      • requirement satisfiedCode-Review
      • requirement satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      • requirement satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
      Gerrit-Change-Number: 804040
      Gerrit-PatchSet: 3
      Gerrit-Owner: Nicholas Husin <n...@golang.org>
      Gerrit-Reviewer: Daniel McCarney <dan...@binaryparadox.net>
      Gerrit-Reviewer: Neal Patel <ne...@golang.org>
      Gerrit-Reviewer: Nicholas Husin <hu...@google.com>
      Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
      Gerrit-Reviewer: Roland Shoemaker <rol...@golang.org>
      Gerrit-Attention: Roland Shoemaker <rol...@golang.org>
      Gerrit-Attention: Neal Patel <ne...@golang.org>
      Gerrit-Attention: Nicholas Husin <n...@golang.org>
      Gerrit-Comment-Date: Wed, 22 Jul 2026 17:49:27 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Roland Shoemaker (Gerrit)

      unread,
      Jul 22, 2026, 2:18:19 PM (4 days ago) Jul 22
      to Nicholas Husin, goph...@pubsubhelper.golang.org, Nicholas Husin, Daniel McCarney, golang...@luci-project-accounts.iam.gserviceaccount.com, Neal Patel, golang-co...@googlegroups.com
      Attention needed from Neal Patel and Nicholas Husin

      Roland Shoemaker voted Code-Review+2

      Code-Review+2
      Open in Gerrit

      Related details

      Attention is currently required from:
      • Neal Patel
      • Nicholas Husin
      Submit Requirements:
        • requirement satisfiedCode-Review
        • requirement satisfiedNo-Unresolved-Comments
        • requirement satisfiedReview-Enforcement
        • requirement satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
        Gerrit-Change-Number: 804040
        Gerrit-PatchSet: 3
        Gerrit-Owner: Nicholas Husin <n...@golang.org>
        Gerrit-Reviewer: Daniel McCarney <dan...@binaryparadox.net>
        Gerrit-Reviewer: Neal Patel <ne...@golang.org>
        Gerrit-Reviewer: Nicholas Husin <hu...@google.com>
        Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
        Gerrit-Reviewer: Roland Shoemaker <rol...@golang.org>
        Gerrit-Attention: Neal Patel <ne...@golang.org>
        Gerrit-Attention: Nicholas Husin <n...@golang.org>
        Gerrit-Comment-Date: Wed, 22 Jul 2026 18:18:14 +0000
        Gerrit-HasComments: No
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        open
        diffy

        Nicholas Husin (Gerrit)

        unread,
        Jul 22, 2026, 2:18:47 PM (4 days ago) Jul 22
        to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Roland Shoemaker, Nicholas Husin, Daniel McCarney, golang...@luci-project-accounts.iam.gserviceaccount.com, Neal Patel, golang-co...@googlegroups.com

        Nicholas Husin submitted the change

        Change information

        Commit message:
        crypto/tls: avoid overflow in parseECHConfigList

        When parsing an ECHConfigList, length fields were previously evaluated
        as a uint16. This would cause an infinite loop to occur when parsing a
        65532 bytes long ECHConfig with a declared outer length header of 0.

        Triggering this condition requires a payload of at least 65538 bytes. As
        ECHConfigList payloads are typically delivered via protocols that limit
        them to 65535 bytes (DNS HTTPS records and TLS extensions), regular
        clients are safe from this issue.

        Thank you to Nguyễn Hoàng Hải (facebook...@gmail.com) for reporting
        this issue.

        Fixes #80513
        Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
        Reviewed-by: Daniel McCarney <dan...@binaryparadox.net>
        Reviewed-by: Nicholas Husin <hu...@google.com>
        Reviewed-by: Roland Shoemaker <rol...@golang.org>
        Files:
        • M src/crypto/tls/ech.go
        • M src/crypto/tls/ech_test.go
        Change size: S
        Delta: 2 files changed, 21 insertions(+), 3 deletions(-)
        Branch: refs/heads/master
        Submit Requirements:
        Open in Gerrit
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: merged
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I8c5011d3b375c24794dd38b915bb4dc06a6a6964
        Gerrit-Change-Number: 804040
        Gerrit-PatchSet: 4
        open
        diffy
        satisfied_requirement
        Reply all
        Reply to author
        Forward
        0 new messages