Opened 8 years ago

Closed 22 months 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


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);


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)

buffer.c.patch (292 bytes) - added by dogbert2 8 years ago.
Patch file for ticket 586

Download all attachments as: .zip

Change History (10)

Changed 8 years ago by dogbert2

Attachment: buffer.c.patch added

Patch file for ticket 586

comment:1 Changed 8 years ago by Gert Döring

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 7 years ago by Gert Döring

Milestone: release 2.3.8release 2.3.9

comment:3 Changed 7 years ago by Samuli Seppänen

Milestone: release 2.3.9release 2.3.12

comment:4 Changed 6 years ago by Gert Döring

Milestone: release 2.3.12release 2.3.14
Owner: set to David Sommerseth
Status: newassigned

either we rip out DMALLOC or we fix it :)

comment:5 Changed 5 years ago by Playgame123

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 and save i must be most people have the concoct this latest version.

Version 0, edited 5 years ago by Playgame123 (next)

comment:6 Changed 3 years ago by tct

Cc: tct added

comment:7 Changed 3 years ago by nomozine

Thank you for this information

Very helpful

comment:8 Changed 22 months ago by Gert Döring

Milestone: release 2.3.14release 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);
            ret = calloc(1, n);

so it seems we should really clean this up one day...

comment:9 Changed 22 months ago by Gert Döring

Milestone: release 2.5.3release 2.5.2
Resolution: fixed
Status: assignedclosed

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

Note: See TracTickets for help on using tickets.