Nice catch, Lev. The patch indeed fixes an UAC. Compiled and tested it with MSVC.
Acked-by: Simon Rozman <si...@rozman.si>
Regards, Simon
> -----Original Message-----
> From: Lev Stipakov <lstipa...@gmail.com>
> Sent: Thursday, March 12, 2020 7:08 AM
> To: openvpn-devel@lists.sourceforge.net
> Cc: Lev Stipakov <l...@openvpn.net>
> Subject: [Openvpn-devel] [PATCH] tun.c: fix "use after free" error
>
> From: Lev Stipakov <l...@openvpn.net>
>
> Commit 509c45f has factored out code blocks of open_tun() into separate
> functions and introduced "use after free" bug:
>
> Variable "device_guid" is allocated inside tun_open_device() function
> and used outside of it. Allocation happens with local gc_arena, which is
> freed at the end of tun_open_device(), making futher access to
> "device_guid" invalid.
>
> Fix by ensuring that gc_arena scope covers all access to "device_guid".
>
> Signed-off-by: Lev Stipakov <l...@openvpn.net>
> ---
> src/openvpn/tun.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index
> 42193d97..c976055e 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -6226,12 +6226,11 @@ tun_try_open_device(struct tuntap *tt, const
> char *device_guid, const struct dev }
>
> static void
> -tun_open_device(struct tuntap *tt, const char *dev_node, const char
> **device_guid)
> +tun_open_device(struct tuntap *tt, const char *dev_node, const char
> +**device_guid, struct gc_arena *gc)
> {
> - struct gc_arena gc = gc_new();
> - const struct tap_reg *tap_reg = get_tap_reg(&gc);
> - const struct panel_reg *panel_reg = get_panel_reg(&gc);
> - const struct device_instance_id_interface
> *device_instance_id_interface = get_device_instance_id_interface(&gc);
> + const struct tap_reg *tap_reg = get_tap_reg(gc);
> + const struct panel_reg *panel_reg = get_panel_reg(gc);
> + const struct device_instance_id_interface
> + *device_instance_id_interface = get_device_instance_id_interface(gc);
> char actual_buffer[256];
>
> at_least_one_tap_win(tap_reg);
> @@ -6244,7 +6243,7 @@ tun_open_device(struct tuntap *tt, const char
> *dev_node, const char **device_gui
> enum windows_driver_type windows_driver =
> WINDOWS_DRIVER_UNSPECIFIED;
>
> /* Get the device GUID for the device specified with --dev-
> node. */
> - *device_guid = get_device_guid(dev_node, actual_buffer,
> sizeof(actual_buffer), &windows_driver, tap_reg, panel_reg, &gc);
> + *device_guid = get_device_guid(dev_node, actual_buffer,
> + sizeof(actual_buffer), &windows_driver, tap_reg, panel_reg, gc);
>
> if (!*device_guid)
> {
> @@ -6276,7 +6275,7 @@ tun_open_device(struct tuntap *tt, const char
> *dev_node, const char **device_gui
> tap_reg,
> panel_reg,
> &windows_driver,
> - &gc);
> + gc);
>
> if (!*device_guid)
> {
> @@ -6304,8 +6303,6 @@ next:
>
> msg(M_INFO, "%s device [%s] opened", print_windows_driver(tt-
> >windows_driver), tt->actual_name);
> tt->adapter_index = get_adapter_index(*device_guid);
> -
> - gc_free(&gc);
> }
>
> static void
> @@ -6411,13 +6408,16 @@ open_tun(const char *dev, const char *dev_type,
> const char *dev_node, struct tun
> msg(M_FATAL|M_NOPREFIX, "Unknown virtual device type: '%s'",
> dev);
> }
>
> - tun_open_device(tt, dev_node, &device_guid);
> + struct gc_arena gc = gc_new(); /* used also for device_guid
> allocation */
> + tun_open_device(tt, dev_node, &device_guid, &gc);
>
> if (tt->windows_driver == WINDOWS_DRIVER_TAP_WINDOWS6)
> {
> tuntap_post_open(tt, device_guid);
> }
>
> + gc_free(&gc);
> +
> /*netcmd_semaphore_release ();*/
> }
>
> --
> 2.17.1
>
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
