[webkit-reviews] review granted: [Bug 46450] Add Windows implementation of PlatformCertificateInfo : [Attachment 68653] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Sam Weinig
<sam at webkit.org>'s request for review:
Bug 46450: Add Windows implementation of PlatformCertificateInfo
https://bugs.webkit.org/show_bug.cgi?id=46450

Attachment 68653: Patch
https://bugs.webkit.org/attachment.cgi?id=68653&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=68653&action=review

> WebKit2/Shared/win/PlatformCertificateInfo.cpp:54
> +    RetainPtr<CFDictionaryRef> certificateInfo =
wkGetSSLCertificateInfo(cfResponse);
> +    if (!certificateInfo)
> +	   return;

Why the RetainPtr? There's no need to retain/release the dictionary in this
function.

> WebKit2/Shared/win/PlatformCertificateInfo.cpp:60
> +    m_certificateContext =
CertDuplicateCertificateContext((PCCERT_CONTEXT)data);

static_cast would be nicer.

Seems like this file needs a sprinkling of :: on the CryptoAPI calls.

> WebKit2/Shared/win/PlatformCertificateInfo.cpp:118
> +    nameSize = CertGetNameString(certificate, dwType, 0, pvTypePara, 0, 0);
> +    if (!nameSize)
> +	   return 0;
> +    OwnArrayPtr<WCHAR> name(new WCHAR[nameSize - 1]);
> +    CertGetNameString(certificate, dwType, 0, pvTypePara, name.get(),
nameSize);

I think you're causing a buffer overrun here. You're telling CertGetNameString
that the buffer is nameSize characters long, but it's only nameSize - 1
characters long!

You should also use ::CertGetNameStringW.