Merge pull request #6308 from mook-as/win32/certs/copy-unsafe

WSL-Helper: Certs: Make a copy of foreign memory

.github/actions/spelling/expect.txt

@@ -243,6 +243,7 @@ fav
 fcb
 fdx
 featurename
+FEEEFEEE
 femto
 ffi
 ficlone

src/go/wsl-helper/pkg/certificates/certificates_windows.go

@@ -74,7 +74,13 @@ func GetSystemCertificates(storeName string) (<-chan Entry, error) {
  }
  break
  }
- cert, err := x509.ParseCertificate(unsafe.Slice(certCtx.EncodedCert, certCtx.Length))
+ // Make a copy of the encoded cert, because the parsed cert may have
+ // references to the memory (that isn't owned by the GC) and we'll return
+ // it in a channel, so HeapFree() might get called on it before it's used.
+ // See #6295 / #6307.
+ certData := make([]byte, certCtx.Length)
+ copy(certData, unsafe.Slice(certCtx.EncodedCert, certCtx.Length))
+ cert, err := x509.ParseCertificate(certData)
  if err != nil {
  // Skip invalid certs
  logrus.Tracef("Skipping invalid certificate %q in %q: %s", getCertName(certCtx), storeName, err)

src/go/wsl-helper/pkg/certificates/certificates_windows_test.go

@@ -0,0 +1,47 @@
+package certificates_test
+
+import (
+ "bytes"
+ "crypto/x509"
+ "encoding/pem"
+ "testing"
+
+ "github.com/rancher-sandbox/rancher-desktop/src/go/wsl-helper/pkg/certificates"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// Test that we don't use memory that we don't own
+func TestGetSystemCertificates_UseAfterFree(t *testing.T) {
+ var certs []*x509.Certificate
+ ch, err := certificates.GetSystemCertificates("CA")
+ require.NoError(t, err, "failed to get CA certificates")
+ for entry := range ch {
+ if assert.NoError(t, err, entry.Err) {
+ certs = append(certs, entry.Cert)
+ }
+ }
+ ch, err = certificates.GetSystemCertificates("ROOT")
+ require.NoError(t, err, "failed to get ROOT certificates")
+ for entry := range ch {
+ if assert.NoError(t, err, entry.Err) {
+ certs = append(certs, entry.Cert)
+ }
+ }
+
+ // By this point, both channels have been closed, which also means we have
+ // closed both cert stores.
+ for _, cert := range certs {
+ buf := bytes.Buffer{}
+ block := &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}
+ err = pem.Encode(&buf, block)
+ if assert.NoError(t, err, "Failed to encode certificate") {
+ // Look for invalid certificates:
+ // - A line of all A (nulls)
+ // - A line with 0xFEEEFEEE (HeapAlloc freed marker)
+ output := buf.String()
+ assert.NotContains(t, output, "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", "encoded cert contains nulls")
+ assert.NotContains(t, output, "7v7u/u7+7v7u/u7+7v7u/u7+7v7u/u7+7v7u/u7+7v7u/u7+7v7u/u7+7v7u/u7+", "encoded cert contains FEEEFEEE")
+ }
+ }
+}