[Bug 38895] New: IrfanView cannot use its WebP plugin
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(a)winehq.org Reporter: t.artem(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Artem S. Tashkinov <t.artem(a)mailcity.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, regression -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Austin English <austinenglish(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|critical |normal --- Comment #1 from Austin English <austinenglish(a)gmail.com> --- Not critical. Please run a regression test: http://wiki.winehq.org/RegressionTesting -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 --- Comment #2 from Artem S. Tashkinov <t.artem(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Artem S. Tashkinov <t.artem(a)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(a)mailcity.com> --- 7e1c886fbfd362376b6aebe5381ab7d4433c3371 is the first bad commit commit 7e1c886fbfd362376b6aebe5381ab7d4433c3371 Author: André Hentschel <nerv(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Sebastian Lackner <sebastian(a)fds-team.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nerv(a)dawncrow.de, | |sebastian(a)fds-team.de Regression SHA1| |7e1c886fbfd362376b6aebe5381 | |ab7d4433c3371 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Artem S. Tashkinov <t.artem(a)mailcity.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38897 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Erich E. Hoover <erich.e.hoover(a)wine-staging.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |erich.e.hoover(a)wine-staging | |.com --- Comment #4 from Erich E. Hoover <erich.e.hoover(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 --- Comment #5 from Artem S. Tashkinov <t.artem(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Anastasius Focht <focht(a)gmx.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEW CC| |focht(a)gmx.net Ever confirmed|0 |1 --- Comment #6 from Anastasius Focht <focht(a)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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Anastasius Focht <focht(a)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(a)gmx.net> --- Hello folks, refining the summary to prepare for collection. Also raising severity as this potentially affects a wide range of apps. Regards -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Rosanne DiMesio <dimesio(a)earthlink.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dimesio(a)earthlink.net --- Comment #8 from Rosanne DiMesio <dimesio(a)earthlink.net> --- *** Bug 38897 has been marked as a duplicate of this bug. *** -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 --- Comment #9 from Erich E. Hoover <erich.e.hoover(a)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? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 --- Comment #10 from Anastasius Focht <focht(a)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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Erich E. Hoover <erich.e.hoover(a)wine-staging.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51832|0 |1 is obsolete| | --- Comment #11 from Erich E. Hoover <erich.e.hoover(a)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 ;) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 --- Comment #12 from André H. <nerv(a)dawncrow.de> --- Sorry for the late reply, i guess this totally destroys my JR ;) Thanks Erich for picking it up that quickly! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 --- Comment #13 from Erich E. Hoover <erich.e.hoover(a)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? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 --- Comment #14 from Sebastian Lackner <sebastian(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Michael Müller <michael(a)fds-team.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |michael(a)fds-team.de -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Erich E. Hoover <erich.e.hoover(a)wine-staging.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51833|0 |1 is obsolete| | --- Comment #15 from Erich E. Hoover <erich.e.hoover(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 --- Comment #16 from Dmitry Timoshkov <dmitry(a)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 ? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Ken Sharp <imwellcushtymelike(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|38897 | -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 André H. <nerv(a)dawncrow.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |e9d7cf99ada80ea8345c301481c | |63a24780f2b63 Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #17 from André H. <nerv(a)dawncrow.de> --- Should be fixed by http://source.winehq.org/git/wine.git/commitdiff/e9d7cf99ada80ea8345c301481c... Thanks Erich -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Anastasius Focht <focht(a)gmx.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nanawel(a)gmail.com --- Comment #18 from Anastasius Focht <focht(a)gmx.net> --- *** Bug 38934 has been marked as a duplicate of this bug. *** -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Sebastian Lackner <sebastian(a)fds-team.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=38949 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #19 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 1.7.48. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=38895 Anastasius Focht <focht(a)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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (2)
-
wine-bugs＠winehq.org -
WineHQ Bugzilla