Opened 9 years ago
Closed 3 years ago
#586 closed Patch submission (fixed)
Possible Segmentation Fault issue in OpenVPN 2.3.x
Reported by: | dogbert2 | Owned by: | David Sommerseth |
---|---|---|---|
Priority: | major | Milestone: | release 2.5.2 |
Component: | Generic / unclassified | Version: | OpenVPN 2.3.5 (Community Ed) |
Severity: | Not set (select this one, unless your'e a OpenVPN developer) | Keywords: | Possible Segmentation Violation/Fault |
Cc: | David Sommerseth, tct |
Description
Hello All,
In reviewing code in OpenVPN 2.3.6, I found in file 'buffer.c'
if DMALLOC is defined, at line 553, where ret = openvpn_dmalloc
(file, line, n) which is a call to malloc() in the memdbg.h file
where it appears to reference malloc(). In the case where NULL
is returned to variable 'ret', in this case, a call to memset()
with variable 'ret' equal to NULL, which will result in a
segmentation violation/fault.
The patch file below should address this issue:
--- buffer.c.orig 2015-07-31 23:03:56.265948284 -0400
+++ buffer.c 2015-07-31 23:05:21.439948284 -0400
@@ -554,6 +554,7 @@
*/
#ifdef DMALLOC
ret = openvpn_dmalloc (file, line, n);
+ check_malloc_return(ret);
memset(ret, 0, n);
#else
ret = calloc(1, n);
Comments, Questions, Suggestions, Complaints? :)
I am attaching the patch file to this bug report...
Bill Parker (wp02855 at gmail dot com)
Attachments (1)
Change History (10)
Changed 9 years ago by
Attachment: | buffer.c.patch added |
---|
comment:1 Changed 9 years ago by
Cc: | David Sommerseth added |
---|---|
Milestone: | → release 2.3.8 |
It's a somewhat unlikely case and would need an actual DMALLOC build to be risky - but still, as long as we're not sure that dmalloc_malloc() (which openvpn_dmalloc() maps to) never returns NULL, needs fixing...
The problematic code was introduced in commit dc7be6d0, part of a series of ambitious but not well-understood-enough cleanup changes.
comment:2 Changed 9 years ago by
Milestone: | release 2.3.8 → release 2.3.9 |
---|
comment:3 Changed 8 years ago by
Milestone: | release 2.3.9 → release 2.3.12 |
---|
comment:4 Changed 8 years ago by
Milestone: | release 2.3.12 → release 2.3.14 |
---|---|
Owner: | set to David Sommerseth |
Status: | new → assigned |
either we rip out DMALLOC or we fix it :)
comment:5 Changed 6 years ago by
My computer is the best platform to given the commend easily in your desktop connection by the users you see the file the explorer in http://mycomputerwindows10.com and save i must be most people have the concoct this latest version.
comment:6 Changed 5 years ago by
Cc: | tct added |
---|
comment:8 Changed 3 years ago by
Milestone: | release 2.3.14 → release 2.5.3 |
---|
Not sure why this was never closed.
Checked buffer.c, and while there's check_malloc_return() all over the place, the problematic code is still there
#ifdef DMALLOC ret = openvpn_dmalloc(file, line, n); memset(ret, 0, n); #else ret = calloc(1, n); #endif check_malloc_return(ret);
so it seems we should really clean this up one day...
comment:9 Changed 3 years ago by
Milestone: | release 2.5.3 → release 2.5.2 |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
commit e2acfad40c0d79ce7fd431c380d7466d383bcefa (master)
commit acf52dda9f4cb117e9d020dd06fccd7ecb90d303 (release/2.5)
commit a7263a125199c6d11710ecf50f9a07424369fdbc (release/2.4)
Author: Gert Doering
Date: Fri Apr 2 19:34:14 2021 +0200
Fix potential NULL ptr crash if compiled with DMALLOC
Patch file for ticket 586