New dsniff revision

[James Cowgill]

Hi,

On 22/02/17 23:16, Marcos Fouces wrote:
> Hello
> 
> I uploaded to repo a first attempt of libnids1.24 package wich aims to
> close its two critical bugs:
> 
> #855602: fixed upstream in 1.24 release.

As I wrote in the bugreport, the fix applied by upstream is not enough
to fix this. You also need to apply the patch from Fedora otherwise
dsniff will FTBFS on i386.

> #851060: i tried using -fno-strict-aliasing flag as reporter indicates.
> I didn't noticed any change on my amd64, but he talked about armhf arch.
>
> I built dsniff (1) against this new version of libnids-dev (2) and here
> is the result:
> 
> * Run dsniff
> 
> * Run the same test as bug reporter: curl -v --basic --user foo:bar
> http://neverssl.com/
> 
> dsniff says this:
> 
> dsniff: listening on eth0
> -----------------
> 02/22/17 23:56:32 tcp 192.168.1.3.47942 ->
> s3-website-us-west-2.amazonaws.com.80 (http)
> GET / HTTP/1.1
> Host: neverssl.com
> Authorization: Basic Zm9vOmJhcg== [foo:bar]

So interestingly, after recompiling your new versions of both libnids
and dsniff, things start working again. I'm not sure what happened
before when I couldn't get anything to work.

> (1) https://anonscm.debian.org/cgit/pkg-security/dsniff.git

For stretch, I don't think dsniff needs to be changed.

> (2) https://anonscm.debian.org/cgit/pkg-security/libnids.git

At the moment Debian is frozen and you've made a number of changes here
which are not sutible for the freeze. These include:
- Packaging a new version of libnids.
- Breaking the ABI of libnids.
- Bumping debhelper compat

For stretch, the fixes need to be targeted so as to only change as much
stuff as necessary to fix the two RC bugs.

For buster, can I suggest you rewrite the rules file. I expect it could
be simplified much more using dh.

Looking at the diff from upstream, I can't help but stare in disbelief
at this:
> diff --git a/src/hash.c b/src/hash.c
> index 7e4c611..3eb28ca 100644
> --- a/src/hash.c
> +++ b/src/hash.c
> @@ -57,7 +57,8 @@ mkhash (u_int src, u_short sport, u_int dest,
u_short dport)
>    u_int res = 0;
>    int i;
>    u_char data[12];
> -  *(u_int *) (data) = src;
> +  u_int *stupid_strict_aliasing_warnings=(u_int*)data;
> +  *stupid_strict_aliasing_warnings = src;
>    *(u_int *) (data + 4) = dest;
>    *(u_short *) (data + 8) = sport;
>    *(u_short *) (data + 10) = dport;

A minor note: as someone not part of the pkg-security team, I find it
confusing to have both a "debian/master" and "master" branch in the same
repository both referring to Debian packaging. I think you should remove
the "master" branch (unless this is a team policy?)

Thanks,
James

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-security-team/attachments/20170223/7904bb76/attachment.sig>