Opened 4 years ago

Closed 4 years ago

#779 closed Bug / Defect (fixed)

openvpn --setenv opt segfaults

Reported by: Selva Nair Owned by:
Priority: major Milestone: release 2.4
Component: Generic / unclassified Version: OpenVPN 2.4_alpha2 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

openvpn --setenv opt without a second argument segfaults on Linux and Windows and may be other platforms.

version 2.4_beta2 (tagged as alpha2 as I cant find beta2 in the drop down menu).

P.S.
It seems older version segfaults too, so this is not 2.4-specific.

Change History (3)

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

Indeed, we broke that somewhen in 2.3...

gert@fbsd74:/root/t_server$ ./openvpn.22 --setenv opt
Options error: You must define TUN/TAP device (--dev)
Use --help for more information.
gert@fbsd74:/root/t_server$ ./openvpn.23 --setenv opt
Memory fault
gert@fbsd74:/root/t_server$ ./openvpn.master --setenv opt
Memory fault

Since this cannot be pushed (phew!)...

Tue Nov 29 21:21:55 2016 Options error: option 'setenv' cannot be used in this context ([PUSH-OPTIONS])

... this is not something that "must! absolutely!" be fixed for 2.4 release... but it's good you found it. Segfaults are always embarrassing... wonder who broke that :-)

comment:2 Changed 4 years ago by Selva Nair

2.2 probably did not have setenv-opt. The first commit seems to be
commit 2a92fba756d4c1e73300a12ff9e80028a6ab7c09
Author: James Yonan <james@…>
Date: Tue Jun 11 00:25:05 2013 -0600

which is apparently cherry-picked from commit 27713761e4110bb92f1c6dfe85db291e8c6e0f56 but that commit is not in my tree..

Anyway, bikeshedding here as this is not critical. The fix is easy though:

if (streq (p[0], "setenv") && p[1] && streq (p[1], "opt") && !(permission_mask & OPT_P_PULL_MODE))
  {
      p += 2;
      msglevel_fc = M_WARN;
  }

should check that p[2] is not null before doing p += 2. Else the new options head, p[0], will be NULL and get dereferenced in subsequent parsing using streq()'s.

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

Milestone: release 2.4
Resolution: fixed
Status: newclosed

commit 997795353916ffcb413a2da02dc7f210fd621954 (master)
commit 290cc3f8d50435a6ed5f2cb1ecd9056dadcc4783 (release/2.3)
Author: Selva Nair
Date: Tue Nov 29 20:53:14 2016 -0500

When parsing '--setenv opt xx ..' make sure a third parameter is present

thanks :)

Note: See TracTickets for help on using tickets.