I am programming in win32 for years, still today.

The patch seems relevant, compliant with MS guidelines on Bitmaps manipulations.

Anyway, it is documented that other memory allocation than VirtualAlloc for use with getdibits is not safe :
http://msdn.microsoft.com/en-us/library/windows/desktop/dd144879%28v=vs.85%29.aspx

Personnally, I can state that on MS Windows including w8, malloc is not MT safe (while HeapAlloc is; but unfortunately I did not test with VirtualAlloc).
Ok...I know that openssl itself is not MT safe today.

Is Openssl_ALLOC safer than all this MS stuff ? hope so.

Original code (NOT patched) had a little mistake by restoring a useless bitmap in a useless memory DC.
The patch fix this.

Patched and original code are just seeding the random generator with multiple hashes computed from 16 lines blocks taken from the screen : is it enough when it is not guaranteed that the screen is changing from time to time, eg on non interactive machines (servers) ? is it enough on small devices such as phones, most of the time displaying the same "main screen" ?

Anyway, I am not an openssl expert to explain why the code is acting by computing many hashes on small block of lines of the screen,
instead of computing one single hash on the complete screen:
is it to save memory consumption ?not really a problem these days as screens still have limited resolution compared to the PC amount of memory... is is to get some "more" entropy on randomization ? not really relevant to my point of view.

For the screen capture method : there are alternatives, eg based on createDIBsection, that may give even more compact code and avoid questioning about OPENSSL_ALLOC.

GetDibits COULD also have returned itself the proper buffer, if called 2 times, first with lpvbits at NULL and then with the first call returned value.

A suggestion : on modern PC or phones, something can give much more entropy than the screen : the camera with its natural electronic noise...but using it without the consent of the user is not correct...
Or the sound speakers...but not correct either...
anyway, from a consent point of view, it is the same problem as capturing the PC screen without the consent of the user.


**WCE :
The patched code, as the original, are NOT WCE compatible :
for WCE, all the routine is empty'ed...is it a good behavior ?...I do not think so...

If needed I CAN offer a WCE version.

Yours sincerely
Pierre Delaage




Le 14/09/2014 21:05, Salz, Rich a écrit :

Any input from Windows folks on the attached?

--

Principal Security Engineer, Akamai Technologies

IM: rs...@jabber.me <mailto:rs...@jabber.me> Twitter: RichSalz


Reply via email to