Runtime error in encoding/asn1

[lu4p]

Code to reproduce (simple PEM certificate decoding):

package main

import (
	"crypto/x509"
	"encoding/pem"
)

const serverCert = `-----BEGIN CERTIFICATE-----
MIIFUzCCAzugAwIBAgIRAM8OTAqUZdc9eIiE3XvoiaYwDQYJKoZIhvcNAQELBQAw
EjEQMA4GA1UEChMHQWNtZSBDbzAeFw0yMTA3MjAyMjExMjBaFw0zMTA3MTgyMjEx
MjBaMBIxEDAOBgNVBAoTB0FjbWUgQ28wggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAw
ggIKAoICAQDRkqghJKzLhAyjbATi7kWc/69QQoR45JiWAYx7j+V/nS/XZ2/j7Wt0
9Eu45PSQ6OShSum9wa3gfhHL71ub3XtHaUhS1RFq3beEu/t8NrQBYA4mZpWn5neG
5j8LCneb1CwZhGKFyndjIR3V/86q7GLrVE62RoB8ye35w/71tPwc9+QVieHnzMMC
N3Uw/L3hrG3QP4rD2SOHe6qLVf4YCnhmjaEs9CVzu+P3EKhG/PCb53ANwJSl90vj
bzw8hJ08W6fd7keN6h9sIIAao/fM19o1hGTvNfKg9iyqITWr/GkfNNq64lFCPc4W
zlexpKpmoMAXTRi3YYRtPraPERzDuJJryZogpY5O6GNzry7xv0UhI1P0qm1B3x8n
t9hLURfHvcc9vtPdzH/vjjQdz+pS32fvrNVUaZyagP8++JI9nPRFsjNL+JiQr7Vo
dVui41tZegyYd65rbpRmI6ijjLN5otCKPO++q4ZN0FlZeB2m0fMo2AinsFEjzNNp
3AM8s65bnRKc1sjYXjTygZKUiNaMZZdsoq8VsQev8OzQEkw0DXhqvLa4cr2Nro//
jjI+bUnrCxzIE853SLVyeUx8i/jS8UzvYgQjoFZm1JK1yBV/yghLfKFZnENJzDTv
GJNGHgqPhafx71yy430EbvwLvEpRMIdtoQ7erbcKH9GqKn9HlQVdJwIDAQABo4Gj
MIGgMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggrBgEFBQcDATAPBgNVHRMB
Af8EBTADAQH/MB0GA1UdDgQWBBRQyZ7lgtso4jWvcFWF0buYSJcVYzBJBgNVHREE
QjBAgj5pNmFrdHV4Y2tvY3VhcDd6ZGh3cDQ1eml0NmlyNHNvNWh4eGdqcm82amFp
bWs0NGdvcWF1YjN5ZC5vbmlvbjANBgkqhkiG9w0BAQsFAAOCAgEAQC+BdGmXbP5u
u5Rd5jISOiiXBOn8Mj9uNMjs75LzcxruCdBktnw+JkUkNJn0f1qS7caCEzjyOitX
+/VC0Pa+lZuyoaiYfWk27d892jbU7zG4/kwq88Y2du/qN0E/+aEq/mXcANI5nPg5
fNAtkgsMcqCq0ns2ATEh1DrDP0Rp3bZnP923fcCKDPcj6nCKcA5puMLo7C65Vztt
2gR4tVeV/CNlvvlcWGjKS0yQJMMdlCstNnl0A8J68fptgTGYVdxwgHr0HIJSMfPV
z/9ESjj5cB39AaL5eavAj3CUYlfA9P1uKt6jRl6d39NrPBrSVSD2bnKFbqhzggyB
I4Rh0XPI/pTdJZ4L89NXzMftrXrxNIj5l5Vri4hFP1jBhT0Sd9on0G3na3WeKW4V
heYzTNR7ULt4UysPhNdsb1jL/4wjUfgGmWHNzkfrjuETaNFpuiGvsQDpfED3FQ5/
4GBg32bBiFkrNtNDMPbfaTXhVJ+reZ//ERlh6RD0mr7ZvdjDG5ZK+87z9/cTPedI
rAEMRKi3qc+7QWx2p7VvvxLtx/cy7yy4zbKzAYBmvmH5bNAYANKxn1m1lDXWourH
6EU7YEoehXiGkk/5576E182zoVS5k20OSpFkCBuAxZhAuumAYl9nFwrhb0JWnI6K
nw5uyoZyzShdehIs3I+M+3QdonpJZt4=
-----END CERTIFICATE-----`

func main() {
	serverBlock, _ := pem.Decode([]byte(serverCert))

	_, err := x509.ParseCertificate(serverBlock.Bytes)
	if err != nil {
		panic(err)
	}
}

Garble Command:

$ GOPRIVATE="*" garble -debugdir=debug build && ./test
panic: asn1: structure error: sequence tag mismatch

goroutine 1 [running]:
main.main()
        o9BARDLq.go:1 +0xc8

Probably something broke because of the heavy use of reflection in encoding/asn1

[mvdan]

I've got good news and bad news. The good news is that this obfuscated program works just fine on Go master (1.17), and it seems like the reason is https://go-review.googlesource.com/c/go/+/274234:

crypto/x509: rewrite certificate parser

Replaces the encoding/asn1 certificate parser with a x/crypto/cryptobyte based parser. This provides a significant increase in performance, mostly due to a reduction of lots of small allocs as well as almost entirely removing reflection.

It's possible that encoding/asn1 is still a little broken when obfuscated with Go 1.17, but I'm inclined to only pay attention to that once someone runs into it, and instead prioritize other bugs. I imagine that very few users would go straight for encoding/asn1 in general.

The bad news is that a proper fix doesn't seem trivial. The stack trace up to where the error occurs is as follows:

encoding/asn1.parseSequenceOf(0xc0000da2a3, 0xa0, 0x2ca, 0x56c360, 0x51a4a0, 0x56c360, 0x52c880, 0x1, 0x29b, 0x0, ...)
	encoding/asn1/asn1.go:632 +0x1e5
encoding/asn1.parseField(0x51a4a0, 0xc0000b8538, 0x197, 0xc0000da008, 0x33b, 0x565, 0x295, 0x101, 0x0, 0xc0000be4a8, ...)
	encoding/asn1/asn1.go:953 +0x22a5
encoding/asn1.parseField(0x5376a0, 0xc0000b8318, 0x199, 0xc0000da004, 0x553, 0x569, 0x0, 0x0, 0x0, 0x0, ...)
	encoding/asn1/asn1.go:937 +0x1ea5
encoding/asn1.parseField(0x52fe80, 0xc0000b8300, 0x199, 0xc0000da000, 0x557, 0x56d, 0x0, 0x0, 0x0, 0x0, ...)
	encoding/asn1/asn1.go:937 +0x1ea5
encoding/asn1.UnmarshalWithParams(0xc0000da000, 0x557, 0x56d, 0x516460, 0xc0000b8300, 0x0, 0x0, 0x0, 0x0, 0x29, ...)
	encoding/asn1/asn1.go:1119 +0x2b3
encoding/asn1.Unmarshal(...)
	encoding/asn1/asn1.go:1092
crypto/x509.ParseCertificate(0xc0000da000, 0x557, 0x56d, 0x3, 0xc0000b5f68, 0x1)
	crypto/x509/x509.go:1549 +0x7d
main.main()
	command-line-arguments/f.go:47 +0x195

In short, x509.ParseCertificate calls asn1.Unmarshal via https://cs.opensource.google/go/go/+/refs/tags/go1.16.6:src/crypto/x509/x509.go;l=1549. Note that asn1.Unmarshal takes an interface{} destination value. The type in question here is *x509.Certificate.

The reflection entrypoint in asn1 is here: https://cs.opensource.google/go/go/+/refs/tags/go1.16.6:src/encoding/asn1/asn1.go;l=1113;drc=refs%2Ftags%2Fgo1.16.6

But since its argument is of type interface{}, we simply don't do anything there.

The proper fix would be to record that asn1.Unmarshal, just like other std APIs like json.Unmarshal xml.Marshal, use reflection on one of their parameters. Once we do that, then the x509.Certificate type would not be obfuscated. That should be covered by #162 :)