https://bugs.winehq.org/show_bug.cgi?id=38895
Bug ID: 38895 Summary: IrfanView cannot use its WebP plugin Product: Wine Version: 1.7.47 Hardware: x86 URL: http://www.irfanview.com/ OS: Linux Status: UNCONFIRMED Severity: critical Priority: P2 Component: -unknown Assignee: wine-bugs@winehq.org Reporter: t.artem@mailcity.com Distribution: Red Hat
Created attachment 51830 --> https://bugs.winehq.org/attachment.cgi?id=51830 Test Image
There's a regression in Wine 1.7.47: IrfanView can no longer use its WebP plugin (it fails to load pictures in this format or save to this format).
Wine 1.7.45 doesn't have this problem.
There are no (new) error messages in Wine 1.7.47.
https://bugs.winehq.org/show_bug.cgi?id=38895
Artem S. Tashkinov t.artem@mailcity.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, regression
https://bugs.winehq.org/show_bug.cgi?id=38895
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|critical |normal
--- Comment #1 from Austin English austinenglish@gmail.com --- Not critical. Please run a regression test: http://wiki.winehq.org/RegressionTesting
https://bugs.winehq.org/show_bug.cgi?id=38895
--- Comment #2 from Artem S. Tashkinov t.artem@mailcity.com --- (In reply to Austin English from comment #1)
Not critical. Please run a regression test: http://wiki.winehq.org/RegressionTesting
If loading and using DLLs is not critical then, yeah, maybe you're right.
https://bugs.winehq.org/show_bug.cgi?id=38895
Artem S. Tashkinov t.artem@mailcity.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|IrfanView cannot use its |[bisected] IrfanView cannot |WebP plugin |use its WebP plugin
--- Comment #3 from Artem S. Tashkinov t.artem@mailcity.com --- 7e1c886fbfd362376b6aebe5381ab7d4433c3371 is the first bad commit commit 7e1c886fbfd362376b6aebe5381ab7d4433c3371 Author: André Hentschel nerv@dawncrow.de Date: Tue Jul 7 19:50:25 2015 +0200
ntdll: Randomize security cookie when available.
:040000 040000 eb0acb52430399cbe1b9957cb712e3491ef17958 ba23d34ab2977cba80866518f886d78fee79d3e8 M dlls
Reverting this patch makes the problem go away.
https://bugs.winehq.org/show_bug.cgi?id=38895
Sebastian Lackner sebastian@fds-team.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |nerv@dawncrow.de, | |sebastian@fds-team.de Regression SHA1| |7e1c886fbfd362376b6aebe5381 | |ab7d4433c3371
https://bugs.winehq.org/show_bug.cgi?id=38895
Artem S. Tashkinov t.artem@mailcity.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38897
https://bugs.winehq.org/show_bug.cgi?id=38895
Erich E. Hoover erich.e.hoover@wine-staging.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |erich.e.hoover@wine-staging | |.com
--- Comment #4 from Erich E. Hoover erich.e.hoover@wine-staging.com --- Created attachment 51832 --> https://bugs.winehq.org/attachment.cgi?id=51832 ntdll: Only set the security cookie if it has not already been set.
Please try the attached patch.
https://bugs.winehq.org/show_bug.cgi?id=38895
--- Comment #5 from Artem S. Tashkinov t.artem@mailcity.com --- (In reply to Erich E. Hoover from comment #4)
Created attachment 51832 [details] ntdll: Only set the security cookie if it has not already been set.
Please try the attached patch.
It fixes the issue, thank you.
https://bugs.winehq.org/show_bug.cgi?id=38895
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEW CC| |focht@gmx.net Ever confirmed|0 |1
--- Comment #6 from Anastasius Focht focht@gmx.net --- Hello folks,
I predict an influx of dupe bug reports if distro packagers don't revert the commit on Wine 1.7.47 Unfortunately I didn't have the time this week to follow the project closely.
Erich's patch hides the real problem by keeping the default init value present in in PE compiled with /GS.
The effective bits of the security cookie depends on the Windows version + Windows bitness + initial cookie value. On 32-bit Windows XP/2003 you need to zero out the high 16-bit word (only 16 bit used). On 32-bit Vista/Win7, 32 bits of the cookie are used unless the "magic" 16-bit init value is given. On 64-bit Windows it's always 48 bit being used (highest 16-bit word zeroed).
This code snippet gives some hints (not perfect but to illustrate):
https://github.com/DynamoRIO/dynamorio/blob/master/core/win32/loader.c#L1924
Regards
https://bugs.winehq.org/show_bug.cgi?id=38895
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Component|-unknown |ntdll Summary|[bisected] IrfanView cannot |Multiple applications fail |use its WebP plugin |to load or crash due to | |incorrect security cookie | |randomization by loader | |(IrfanView WebP plugin, | |Word Viewer 2007) Severity|normal |major
--- Comment #7 from Anastasius Focht focht@gmx.net --- Hello folks,
refining the summary to prepare for collection. Also raising severity as this potentially affects a wide range of apps.
Regards
https://bugs.winehq.org/show_bug.cgi?id=38895
Rosanne DiMesio dimesio@earthlink.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dimesio@earthlink.net
--- Comment #8 from Rosanne DiMesio dimesio@earthlink.net --- *** Bug 38897 has been marked as a duplicate of this bug. ***
https://bugs.winehq.org/show_bug.cgi?id=38895
--- Comment #9 from Erich E. Hoover erich.e.hoover@wine-staging.com --- (In reply to Anastasius Focht from comment #6)
... Erich's patch hides the real problem by keeping the default init value present in in PE compiled with /GS. ...
Hmm, my test FIXME apparently was bad (looked like it hit my test case for some reason).
(In reply to Anastasius Focht from comment #6)
... This code snippet gives some hints (not perfect but to illustrate): https://github.com/DynamoRIO/dynamorio/blob/master/core/win32/loader.c#L1924
What is the license on that code? Is this something I can look at?
https://bugs.winehq.org/show_bug.cgi?id=38895
--- Comment #10 from Anastasius Focht focht@gmx.net --- Hello Erich,
this is the copyright header of the file in the link:
--- quote --- /* ********************************************************** * Copyright (c) 2011-2015 Google, Inc. All rights reserved. * Copyright (c) 2009-2010 Derek Bruening All rights reserved. * **********************************************************/
/* * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: * * * Redistributions of source code must retain the above copyright notice, * this list of conditions and the following disclaimer. * * * Redistributions in binary form must reproduce the above copyright notice, * this list of conditions and the following disclaimer in the documentation * and/or other materials provided with the distribution. * * * Neither the name of VMware, Inc. nor the names of its contributors may be * used to endorse or promote products derived from this software without * specific prior written permission. * * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE * ARE DISCLAIMED. IN NO EVENT SHALL VMWARE, INC. OR CONTRIBUTORS BE LIABLE * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH * DAMAGE. */ --- quote ---
Even without looking at that code, I already told relevant bits. The actual randomization algorithm is not that important.
The default cookie values 0x00002B992DDFA232 (64-bit OS), 0xBB40E64E (32-bit OS), 0xBB40 (16-bit on 32-bit OS) are pretty much well known, no secret at all.
Regards
https://bugs.winehq.org/show_bug.cgi?id=38895
Erich E. Hoover erich.e.hoover@wine-staging.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #51832|0 |1 is obsolete| |
--- Comment #11 from Erich E. Hoover erich.e.hoover@wine-staging.com --- Created attachment 51833 --> https://bugs.winehq.org/attachment.cgi?id=51833 ntdll: Only set the security cookie if it has not already been set (2).
(In reply to Anastasius Focht from comment #10)
... The default cookie values 0x00002B992DDFA232 (64-bit OS), 0xBB40E64E (32-bit OS), 0xBB40 (16-bit on 32-bit OS) are pretty much well known, no secret at all.
Excellent, that's exactly what I needed to know. I've attached an updated patch that contains this knowledge. Based on the crash I got for "wordview" with 0xBB40 when I filled the entire 32-bits I'd say that you're not supposed to use more bits than the default cookie value has ;)
https://bugs.winehq.org/show_bug.cgi?id=38895
--- Comment #12 from André H. nerv@dawncrow.de --- Sorry for the late reply, i guess this totally destroys my JR ;) Thanks Erich for picking it up that quickly!
https://bugs.winehq.org/show_bug.cgi?id=38895
--- Comment #13 from Erich E. Hoover erich.e.hoover@wine-staging.com --- (In reply to André H. from comment #12)
Sorry for the late reply, i guess this totally destroys my JR ;) Thanks Erich for picking it up that quickly!
No problem, does it work in your ARM64 test environment?
https://bugs.winehq.org/show_bug.cgi?id=38895
--- Comment #14 from Sebastian Lackner sebastian@fds-team.de --- (In reply to Erich E. Hoover from comment #11)
Created attachment 51833 [details] ntdll: Only set the security cookie if it has not already been set (2).
You still need to modify *cookie when the random number accidentically matches one of the default cookie initializers. Also, there is no need to repeat the check "cookie != NULL" three times.
I am also unsure if its correct to allow 16bit/32bit cookie initializers on 64-bit. The code linked by Anastasius does allow it for 16-bit, but not for 32-bit.
https://bugs.winehq.org/show_bug.cgi?id=38895
Michael Müller michael@fds-team.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |michael@fds-team.de
https://bugs.winehq.org/show_bug.cgi?id=38895
Erich E. Hoover erich.e.hoover@wine-staging.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #51833|0 |1 is obsolete| |
--- Comment #15 from Erich E. Hoover erich.e.hoover@wine-staging.com --- Created attachment 51837 --> https://bugs.winehq.org/attachment.cgi?id=51837 ntdll: Only set the security cookie if it has not already been set (3).
(In reply to Sebastian Lackner from comment #14)
(In reply to Erich E. Hoover from comment #11)
Created attachment 51833 [details] ntdll: Only set the security cookie if it has not already been set (2).
You still need to modify *cookie when the random number accidentically matches one of the default cookie initializers. Also, there is no need to repeat the check "cookie != NULL" three times.
I am also unsure if its correct to allow 16bit/32bit cookie initializers on 64-bit. The code linked by Anastasius does allow it for 16-bit, but not for 32-bit.
My preference would be to implement it like the newly attached version. I was trying to avoid changing things too much in the other patch, but I think this is clearer. Do we have any other test apps that might have 64-bit cookies? We should be able to improve things later if we run into trouble, but I think this is at least a good start.
https://bugs.winehq.org/show_bug.cgi?id=38895
--- Comment #16 from Dmitry Timoshkov dmitry@baikal.ru --- (In reply to Erich E. Hoover from comment #15)
My preference would be to implement it like the newly attached version. I was trying to avoid changing things too much in the other patch, but I think this is clearer. Do we have any other test apps that might have 64-bit cookies? We should be able to improve things later if we run into trouble, but I think this is at least a good start.
Sounds like it's time to add a test to dlls/kernel32/tests/loader.c ?
https://bugs.winehq.org/show_bug.cgi?id=38895
Ken Sharp imwellcushtymelike@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|38897 |
https://bugs.winehq.org/show_bug.cgi?id=38895
André H. nerv@dawncrow.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |e9d7cf99ada80ea8345c301481c | |63a24780f2b63 Status|NEW |RESOLVED Resolution|--- |FIXED
--- Comment #17 from André H. nerv@dawncrow.de --- Should be fixed by http://source.winehq.org/git/wine.git/commitdiff/e9d7cf99ada80ea8345c301481c...
Thanks Erich
https://bugs.winehq.org/show_bug.cgi?id=38895
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |nanawel@gmail.com
--- Comment #18 from Anastasius Focht focht@gmx.net --- *** Bug 38934 has been marked as a duplicate of this bug. ***
https://bugs.winehq.org/show_bug.cgi?id=38895
Sebastian Lackner sebastian@fds-team.de changed:
What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=38949
https://bugs.winehq.org/show_bug.cgi?id=38895
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #19 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 1.7.48.
https://bugs.winehq.org/show_bug.cgi?id=38895
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- URL|http://www.irfanview.com/ |https://web.archive.org/web | |/20140212100036/http://down | |load.microsoft.com/download | |/6/a/6/6a689355-b155-4fa7-a | |d8a-dfe150fe7ac6/wordview_e | |n-us.exe
-
wine-bugs＠winehq.org -
WineHQ Bugzilla