http://bugs.winehq.org/show_bug.cgi?id=20567
Summary: Uninitialised memory reference in RSAENH_CPImportKey Product: Wine Version: 1.1.32 Platform: PC OS/Version: Linux Status: NEW Keywords: download, source, testcase Severity: normal Priority: P2 Component: rsaenh AssignedTo: wine-bugs@winehq.org ReportedBy: dank@kegel.com
This shows up in Chromium's base_unittests.exe in RSAPrivateKeyUnitTest.ShortIntegers test, and also in the wine conformance tests, http://kegel.com/wine/valgrind/logs/2009-10-30-09.22/vg-rsaenh_rsaenh.txt
Use of uninitialised value of size 4 at desfunc (des.c:1366) by des3_ecb_encrypt (des.c:1478) by encrypt_block_impl (implglue.c:282) by RSAENH_CPEncrypt (rsaenh.c:2173) by CryptEncrypt (crypt.c:1083) by CryptProtectData (protectdata.c:906) by store_key_pair (rsaenh.c:962) by store_key_container_keys (rsaenh.c:1147) by release_and_install_key (rsaenh.c:2644) by import_public_key (rsaenh.c:2790) by import_key (rsaenh.c:2973) by RSAENH_CPImportKey (rsaenh.c:3020) by CryptImportKey (crypt.c:1827) by test_import_export (rsaenh.c:1654) Uninitialised value was created by a client request at mark_block_uninitialized (heap.c:187) by RtlAllocateHeap (heap.c:1429) by store_key_pair (rsaenh.c:953) by store_key_container_keys (rsaenh.c:1147) by release_and_install_key (rsaenh.c:2644) by import_public_key (rsaenh.c:2790) by import_key (rsaenh.c:2973) by RSAENH_CPImportKey (rsaenh.c:3020) by CryptImportKey (crypt.c:1827) by test_import_export (rsaenh.c:1654)
BTW someone should check whether we want to refresh the rsaenh source that was derived from libtomcrypt; there is a two years' newer version at http://libtomcrypt.com/download.html
http://bugs.winehq.org/show_bug.cgi?id=20567
--- Comment #1 from Juan Lang juan_lang@yahoo.com 2009-11-03 11:59:54 --- I sent a patch which fixed the warning here: http://www.winehq.org/pipermail/wine-patches/2008-June/056389.html
It was rejected.
http://bugs.winehq.org/show_bug.cgi?id=20567
--- Comment #2 from Dan Kegel dank@kegel.com 2009-11-03 12:58:22 --- Oh, yeah, I remember that. Was there any discussion of why? I can't find it on wine-devel.
http://bugs.winehq.org/show_bug.cgi?id=20567
--- Comment #3 from Juan Lang juan_lang@yahoo.com 2009-11-03 15:27:57 --- Not that I recall. I suspect Alexandre didn't like the big hammer approach to fixing it.
http://bugs.winehq.org/show_bug.cgi?id=20567
--- Comment #4 from Dan Kegel dank@kegel.com 2009-11-03 15:34:16 --- Maybe we should establish what the smallest set of bytes to zero is :-)
http://bugs.winehq.org/show_bug.cgi?id=20567
--- Comment #5 from Juan Lang juan_lang@yahoo.com 2009-11-04 13:06:33 --- Created an attachment (id=24551) --> (http://bugs.winehq.org/attachment.cgi?id=24551) Patch
Does this patch help?
I believe I see the source of the problem: a key of a given length doesn't necessarily need that many bits to store. Consider the number 2: it's valid as a 32-bit number, but it doesn't need more than 2 bits to store. When exporting a key, the function mp_to_unsigned_bin writes one byte of a number at a time to memory, and divides the number by 8 to discard the byte it just wrote. It stops when the result of the division is zero: there are no significant bytes remaining.
This patch makes sure to set to 0 the insignificant bytes. I don't know why neglecting to do this didn't cause issues before, except if the memory was implicitly zeroed (or unless my understanding is incorrect, which is perhaps more likely.)
http://bugs.winehq.org/show_bug.cgi?id=20567
--- Comment #6 from Dan Kegel dank@kegel.com 2009-11-04 16:45:20 --- That seems to get rid of these two errors:
- Conditional jump or move depends on uninitialised value(s) - at test_import_export (rsaenh.c:1846) - by func_rsaenh (rsaenh.c:2490) - - Conditional jump or move depends on uninitialised value(s) - at winetest_vok (test.h:306) - by winetest_ok (test.h:331) - by test_import_export (rsaenh.c:1846) - by func_rsaenh (rsaenh.c:2490)
but leaves ten others (and the two invalid reads). (I'm not applying any other patches, so if you or someone else sent in other patches to help the other problems, that isn't reflected here.)
http://bugs.winehq.org/show_bug.cgi?id=20567
Juan Lang juan_lang@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #24551|0 |1 is obsolete| |
--- Comment #7 from Juan Lang juan_lang@yahoo.com 2009-11-05 12:16:52 --- (From update of attachment 24551) This patch is incorrect, and zeroes too many bytes. I sent improved ones as part of a series (which also fixes bug 20358), beginning here: http://www.winehq.org/pipermail/wine-patches/2009-November/080944.html
http://bugs.winehq.org/show_bug.cgi?id=20567
--- Comment #8 from Juan Lang juan_lang@yahoo.com 2009-11-05 12:21:11 --- I don't claim the patch series eliminates all rsaenh valgrind errors, because valgrind here doesn't produce very useful results. (It occasionally locks up my machine, so I'm hesitant to debug it much.) I'll have to look forward to seeing new logs once the patches are accepted.
http://bugs.winehq.org/show_bug.cgi?id=20567
Juan Lang juan_lang@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |FIXED
--- Comment #9 from Juan Lang juan_lang@yahoo.com 2009-11-09 15:43:28 --- Fixed by commit 49c11910d8e0856715d14567c48a2489ff92d80a.
http://bugs.winehq.org/show_bug.cgi?id=20567
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #10 from Alexandre Julliard julliard@winehq.org 2009-11-13 12:44:48 --- Closing bugs fixed in 1.1.33.
-
wine-bugs＠winehq.org