| | [ Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]
STR #3166
| Application: | FLTK Library |
|---|
| Status: | 1 - Closed w/Resolution |
|---|
| Priority: | 3 - Moderate, e.g. unable to compile the software |
|---|
| Scope: | 2 - Specific to an operating system |
|---|
| Subsystem: | Core Library |
|---|
| Summary: | BEX64 bug on Win7 in fltk-1.3.x-r10474/test/pixmap_browser.cxx |
|---|
| Version: | 1.3.3 |
|---|
| Created By: | szukw000 |
|---|
| Assigned To: | AlbrechtS |
|---|
| Fix Version: | 1.3.4 (SVN: v10544) |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
Trouble Report Comments:
| |
| #1 | szukw000
01:02 Nov 30, 2014 |
| This has been fast an answer, Albrecht! Debugging is more tedious and slow.
-----------------------------
BEX64:
http://technet.microsoft.com/en-us/library/cc738483%28v=ws.10%29.aspx
EventType BEX
Indicates a buffer overflow (/GS) or DEP exception (BEX64 indicates
a buffer overflow (/GS) or DEP exception on 64-bit versions of Windows)
-----------------------------
The file 'fltk-1.3.x-r10474.dif' shows two things:
1. 'getenv("HOME")' does not work on Win7.
2. The bug is in 'quote_pathname()' resp.
'unquote_pathname()'.
With this diff file 'fl_file_chooser()' works without
crash in 'test/pixmap_browser.cxx'.
winfried | |
| |
| #2 | AlbrechtS
06:54 Nov 30, 2014 |
| Thanks for the patch and the link to the technote of the BEX64 bug.
FTR: I take it the first sentence refers to STR #3162.
http://www.fltk.org/str.php?L3162
Looking at the patch there seems to be much not directly related stuff. I'll split the patch and comment the individual parts separately.
More to follow... | |
| |
| #3 | AlbrechtS
07:15 Nov 30, 2014 |
| I can confirm that there are potential buffer overflows in quote_pathname() and unquote_pathname(), but I also believe that your patch changes more than necessary.
Please test the attached patch "file_chooser.diff" and report if this fixes the BEX64 problem. Please revert all other patches you may have used or use another fresh checkout for testing. I believe that this patch contains everything that is necessary to fix the BEX64 bug, if this is really a buffer overflow. I'd appreciate if you could confirm this with a fresh checkout, test w/o the patch (crash) and test with the patch (no crash).
The patch fixes the potential buffer overflows, although I'm not sure why such an overflow would occur. Can you tell me exactly how you can reproduce the bug with unpatched FLTK? As I said before, the file_chooser demo worked for me w/o a crash (under MinGW), but it might be dependent on some personal directory settings (string lengths) etc..
I'll address the getenv("HOME") and the Fl_Preferences issues in a later posting. | |
| |
| #4 | AlbrechtS
08:06 Nov 30, 2014 |
| Your proposed changes to Fl_Preferences{.H/.cxx} include changing the public prototype. Although (and because) both versions are equivalent I don't want to change this, particularly because it could cause ABI changes e.g. if it resulted in different name mangling.
The hunk:
@@ -679,7 +680,7 @@
}
if ( !v ) v = defaultValue;
if ( v ) strlcpy(text, v, maxSize);
- else text = 0;
+ else text[0] = 0;
return ( v != defaultValue );
}
seems to be correct though. I modified it slightly so that it reads
- else text = 0;
+ else *text = 0;
which again is equivalent and consistent with the function argument, although your syntax would have worked as well.
Thanks for this patch. After having a closer look at this part, it *could* have been the root cause, since the given buffer would contain undefined data and if this was used in [un]quote_pathname() this could have resulted in a buffer overflow.
It would be interesting (I'm curious) if you could test this patch alone and see if it fixes the bug (w/o the patch in file_chooser.diff). | |
| |
| #5 | AlbrechtS
08:55 Nov 30, 2014 |
| Part 3: getenv("HOME") ...
I didn't notice that the home directory was not added to the favorites under Windows. It worked when using MinGW, because HOME is defined in this context. However using a "DOS command window" or Windows Explorer to start Windows programs showed that HOME is not defined.
Your patch contains (among others):
+ if ((home = getenv("C:\\Windows\\Temp")) != NULL) {
I don't think that this contributes anything useful because "C:\\Windows\\Temp" won't be defined as an environment variable.
I changed your patch to use a small static function get_homedir() to be more flexible in changing what is needed. This could probably be defined globally, but for now I left it local (static).
Please see attached patch get_homedir.diff. I tested this under Windows and Linux. This ought to resolve the Windows home directory issue. | |
| |
| #6 | AlbrechtS
08:57 Nov 30, 2014 |
| For convenience and easier testing by others I'm also uploading all single patches as one file. See attached file all.patch. Feedback welcome. | |
| |
| #7 | szukw000
12:14 Nov 30, 2014 |
| I have applied the 'all.patch'.
I have tested the pixmap_browser. The fl_file_chooser(), the favorites and the selection: the whole thing works on Win7 using 'cl'.
winfried | |
| |
| #8 | AlbrechtS
13:39 Nov 30, 2014 |
| Thanks for testing, feedback, and the patch this was based on.
I'll commit this soon... | |
| |
| #9 | AlbrechtS
04:10 Jan 31, 2015 |
| Fixed in Subversion repository. | |
[ Return to Bugs & Features ]
|
| |
