Skip to content

bug: tls.Unmarshal decodes Uint24 fields from the wrong offset #1800

Description

@kajaaz

Hi,

I found a correctness bug in the tls.Unmarshal parser for tls.Uint24 fields.

In tls/tls.go, the uint24Type branch in parseField checks the remaining input at the current offset:

rest := data[offset:]
// ...
case uint24Type:
    if len(rest) < 3 {
        return offset, syntaxError{info.fieldName(), "truncated uint24"}
    }

but then decodes the value from data[0:3] instead of the current field position:

v.SetUint(uint64(data[0])<<16 | uint64(data[1])<<8 | uint64(data[2]))
offset += 3
return offset, nil

This means that a Uint24 field is decoded correctly only when it appears at offset 0. If a struct has any field before the Uint24, the parser validates the correct remaining slice but reads the first three bytes of the whole message.

Minimal Reproducer

package main

import (
    "fmt"

    cttls "github.com/google/certificate-transparency-go/tls"
)

type Example struct {
    Prefix byte
    Value  cttls.Uint24
}

func main() {
    // Prefix = 0x41
    // Value should decode from bytes 1..3: 00 00 01 => 1
    data := []byte{0x41, 0x00, 0x00, 0x01}

    var out Example
    rest, err := cttls.Unmarshal(data, &out)
    if err != nil {
        panic(err)
    }
    if len(rest) != 0 {
        panic(fmt.Sprintf("unexpected trailing data: %d bytes", len(rest)))
    }

    fmt.Printf("decoded Value = %d\n", out.Value)
}

Expected result:

decoded Value = 1

Actual result:

decoded Value = 4259840

4259840 is 0x410000, which shows that the Uint24 was decoded from data[0:3] instead of data[1:4].

Suggested Fix

Decode from rest, not data:

v.SetUint(uint64(rest[0])<<16 | uint64(rest[1])<<8 | uint64(rest[2]))

or equivalently decode from rest[:3].

I do not currently have evidence of security impact in this repository itself, because I did not find a production CT structure using tls.Uint24. However, tls.Unmarshal is a public parser API, and downstream users may rely on Uint24 for length or protocol fields, so this seems worth fixing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions