https://bugs.winehq.org/show_bug.cgi?id=44405
--- Comment #17 from Richard Yao ryao@gentoo.org --- (In reply to Nikolay Sivov from comment #16)
BinaryToBase64W is only used from CryptBinaryToStringW, it's not exported.
I made a mistake when interpreting the backtraces. Our call stack is really:
encodeBase64W BinaryToBase64W CryptBinaryToStringW
We are segfaulting in encodeBase64W, which itself is static and is only called by BinaryToBase64W. That is called by CryptBinaryToStringW.
And 'needed' can't be 0. But anyway, let's wait for the patch to come through.
It must be 0 here because
1. The code in BinaryToBase64W is:
charsNeeded = 0; encodeBase64W(pbBinary, cbBinary, sep, NULL, &charsNeeded);
2. The branch is not being taken because it is segfaulting further down in the function according to the backtrace from Proton:
https://github.com/ValveSoftware/Proton/issues/298
3. The backtrace shows out_buf is NULL and the precise line identified is the first attempt to dereference it.
This feels like bug #44583.
After thinking about this some more, I think there are two issues:
1. The calculation of the value of `needed` is likely incorrect given that the function is trying to write out data when the calculation claims that we need zero data. Someone needs to figure out what the correct calculation is. I agree that wine needs test cases here.
2. Fixing the calculation of `needed` would cause us to return ERROR_INSUFFICIENT_BUFFER, which does not seem right to me. Calling this function with a NULL buffer is to rely on the side effect of *out_buf being updated and my intuition is that such a thing should always return success.
I think both solutions are necessary for correctness, although either one would likely be sufficient to make Rise of Nations stop crashing.