Re: [Openvpn-devel] [PATCH v7-master] Add Windows DNS Leak fix using WFP ('block-outside-dns')

2015-12-04 Thread James Yonan
These may have been fixed by now, but noticed some issues in the 
original patch that was discussed in the OpenVPN-devel IRC meeting 
several weeks ago.


* win_adapter_index_to_luid is declared to return a
  NET_LUID but not all code paths return a value.

* wcscat(svchostpath, L"\\svchost.exe") isn't checking
  for buffer overflow.

* FwpmGetAppIdFromFileName0 must be paired with a
  corresponding FwpmFreeMemory0

James



Re: [Openvpn-devel] [PATCH v7-master] Add Windows DNS Leak fix using WFP ('block-outside-dns')

2015-12-04 Thread ValdikSS
These issues should be fixed. Please check PATCH v7.

On 04.12.2015 04:19, James Yonan wrote:
> These may have been fixed by now, but noticed some issues in the original 
> patch that was discussed in the OpenVPN-devel IRC meeting several weeks ago.
>
> * win_adapter_index_to_luid is declared to return a
>   NET_LUID but not all code paths return a value.
>
> * wcscat(svchostpath, L"\\svchost.exe") isn't checking
>   for buffer overflow.
>
> * FwpmGetAppIdFromFileName0 must be paired with a
>   corresponding FwpmFreeMemory0
>
> James




signature.asc
Description: OpenPGP digital signature


Re: [Openvpn-devel] [PATCH v7-master] Add Windows DNS Leak fix using WFP ('block-outside-dns')

2015-12-04 Thread Selva Nair
Hi,

On Fri, Dec 4, 2015 at 12:14 AM, ValdikSS  wrote:

> These issues should be fixed. Please check PATCH v7.
>

I think you missed the NET_LUID one. I had thought this was fixed earlier,
but v7 still has this issue

On 04.12.2015 04:19, James Yonan wrote:
> > These may have been fixed by now, but noticed some issues in the
> original patch that was discussed in the OpenVPN-devel IRC meeting several
> weeks ago.
> >
> > * win_adapter_index_to_luid is declared to return a
> >   NET_LUID but not all code paths return a value.
>

The relevant code is

+ if (ConvertInterfaceIndexToLuid(index, &tapluid) == NO_ERROR)
+   dmsg (D_LOW, "Tap Luid: %I64d", tapluid.Value);

which leaves tapluid undefined if there was an error.

Selva


Re: [Openvpn-devel] [PATCH v7-master] Add Windows DNS Leak fix using WFP ('block-outside-dns')

2015-12-04 Thread ValdikSS
Indeed.
Pushed fixed version to github repo.
https://github.com/ValdikSS/openvpn-with-patches/commit/287ceb11abfa33ee331ba2651572908cbad008d1

If there is no other remarks, I'll send PATCH v8.

On 04.12.2015 08:50, Selva Nair wrote:
> Hi,
>
> On Fri, Dec 4, 2015 at 12:14 AM, ValdikSS  > wrote:
>
> I think you missed the NET_LUID one. I had thought this was fixed earlier, 
> but v7 still has this issue
>
>
> The relevant code is
>
> + if (ConvertInterfaceIndexToLuid(index, &tapluid) == NO_ERROR)
> +   dmsg (D_LOW, "Tap Luid: %I64d", tapluid.Value);
>
> which leaves tapluid undefined if there was an error.
>
> Selva
>



signature.asc
Description: OpenPGP digital signature