Re: [gentoo-proxy-maint] [PATCH] app-crypt/outguess: resurrect version 0.2

[Michał Górny]

Hi, Chris.

Thanks for exploring the new possibility of review via mailing list. I'm
sorry that this take this long but we're a bit overburdened around here.

On czw, 2017-08-10 at 22:14 -0500, Chris Rorvick wrote:
> Gentoo-Bug: 143897
> Package-Manager: Portage-2.3.5, Repoman-2.3.1
> ---
>  app-crypt/outguess/Manifest                        |  1 +
>  ...utguess-0.2-avoid-implicit-function-usage.patch | 53 ++++++++++++++++++++++
>  .../outguess-0.2-fix-printf-argument-types.patch   | 34 ++++++++++++++
>  ...0.2-fix-segfault-when-encoding-PNM-images.patch | 29 ++++++++++++
>  .../outguess-0.2-pass-struct-size-to-memset.patch  | 25 ++++++++++
>  app-crypt/outguess/metadata.xml                    | 19 ++++++++
>  app-crypt/outguess/outguess-0.2-r1.ebuild          | 27 +++++++++++
>  7 files changed, 188 insertions(+)
>  create mode 100644 app-crypt/outguess/Manifest
>  create mode 100644 app-crypt/outguess/files/outguess-0.2-avoid-implicit-function-usage.patch
>  create mode 100644 app-crypt/outguess/files/outguess-0.2-fix-printf-argument-types.patch
>  create mode 100644 app-crypt/outguess/files/outguess-0.2-fix-segfault-when-encoding-PNM-images.patch
>  create mode 100644 app-crypt/outguess/files/outguess-0.2-pass-struct-size-to-memset.patch
>  create mode 100644 app-crypt/outguess/metadata.xml
>  create mode 100644 app-crypt/outguess/outguess-0.2-r1.ebuild
>
> diff --git a/app-crypt/outguess/Manifest b/app-crypt/outguess/Manifest
> new file mode 100644
> index 00000000000..82f73b4f8d1
> --- /dev/null
> +++ b/app-crypt/outguess/Manifest
> @@ -0,0 +1 @@
> +DIST outguess-0.2.tar.gz 470638 SHA256 2f951ed7b9b9373fae8fe95616d49c83ae246cf53a2b60a82814228515bfa7d6 SHA512 2901e2e8f6495f1483897bfde2a84b658335aa733f3ad16959e6b906dec8675c72f5468a0f60b14d231ff64b7768be2cf06be6bbd833286f07a632a9ca45a1e6 WHIRLPOOL e7dca23e1074aeeba817b0ac57f835ca46bfce423735de4b41badafc95313d28c610b84728c91606f03f87f3c8785e714a2f233fda8a980c19bf0e8ca17ca10d
> diff --git a/app-crypt/outguess/files/outguess-0.2-avoid-implicit-function-usage.patch b/app-crypt/outguess/files/outguess-0.2-avoid-implicit-function-usage.patch
> new file mode 100644
> index 00000000000..dccf3e90338
> --- /dev/null
> +++ b/app-crypt/outguess/files/outguess-0.2-avoid-implicit-function-usage.patch
> @@ -0,0 +1,53 @@
> +From 425982db66dc038ffeb025e0f87f98b9b95e68f3 Mon Sep 17 00:00:00 2001
> +From: Chris Rorvick <chris@×××××××.com>
> +Date: Wed, 31 Aug 2016 23:27:17 -0500
> +Subject: [PATCH] avoid implicit function usage
> +
> +Ensure functions are declared before they are used.
> +---
> + jpeg-6b-steg/jcdctmgr.c | 2 ++
> + jpeg-6b-steg/jdcoefct.c | 2 ++
> + missing/md5.c           | 1 +
> + 3 files changed, 5 insertions(+)
> +
> +diff --git a/jpeg-6b-steg/jcdctmgr.c b/jpeg-6b-steg/jcdctmgr.c
> +index 292648d..b68a7ba 100644
> +--- a/jpeg-6b-steg/jcdctmgr.c
> ++++ b/jpeg-6b-steg/jcdctmgr.c
> +@@ -40,6 +40,8 @@ typedef struct {
> +
> + typedef my_fdct_controller * my_fdct_ptr;
> +
> ++short steg_use_bit (unsigned short temp);
> ++
> +
> + /*
> +  * Initialize for a processing pass.
> +diff --git a/jpeg-6b-steg/jdcoefct.c b/jpeg-6b-steg/jdcoefct.c
> +index 6ffe53f..f38f4d5 100644
> +--- a/jpeg-6b-steg/jdcoefct.c
> ++++ b/jpeg-6b-steg/jdcoefct.c
> +@@ -74,6 +74,8 @@ METHODDEF(int) decompress_smooth_data
> + 	JPP((j_decompress_ptr cinfo, JSAMPIMAGE output_buf));
> + #endif
> +
> ++short steg_use_bit (unsigned short temp);
> ++
> +
> + LOCAL(void)
> + start_iMCU_row (j_decompress_ptr cinfo)
> +diff --git a/missing/md5.c b/missing/md5.c
> +index 7cb1d4d..314548a 100644
> +--- a/missing/md5.c
> ++++ b/missing/md5.c
> +@@ -28,6 +28,7 @@
> + #endif
> +
> + #include "md5.h"
> ++#include <string.h>
> +
> + /* Little-endian byte-swapping routines.  Note that these do not
> +    depend on the size of datatypes such as uint32, nor do they require
> +--
> +2.9.3
> +
> diff --git a/app-crypt/outguess/files/outguess-0.2-fix-printf-argument-types.patch b/app-crypt/outguess/files/outguess-0.2-fix-printf-argument-types.patch
> new file mode 100644
> index 00000000000..3142f590025
> --- /dev/null
> +++ b/app-crypt/outguess/files/outguess-0.2-fix-printf-argument-types.patch
> @@ -0,0 +1,34 @@
> +From fab9e5815766d5ed0434b0ab82f17d2a11c9ad6d Mon Sep 17 00:00:00 2001
> +From: Chris Rorvick <chris@×××××××.com>
> +Date: Wed, 31 Aug 2016 23:55:21 -0500
> +Subject: [PATCH] fix printf argument types
> +
> +---
> + outguess.c | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +diff --git a/outguess.c b/outguess.c
> +index 557be9e..c0448b6 100644
> +--- a/outguess.c
> ++++ b/outguess.c
> +@@ -693,7 +693,7 @@ do_embed(bitmap *bitmap, u_char *filename, u_char *key, u_int klen,
> + 	if (bitmap->maxcorrect && correctlen > bitmap->maxcorrect) {
> + 		fprintf(stderr, "steg_embed: "
> + 			"message larger than correctable size %d > %d\n",
> +-			correctlen, bitmap->maxcorrect);
> ++			(int)correctlen, (int)bitmap->maxcorrect);

Are you sure this is the correct solution here? I think it'd be better
to change the printf format from %d to e.g. %ld? Or one of the
inttypes.h consts if int*_t/uint*_t types are used.

> + 		exit(1);
> + 	}
> +
> +@@ -975,7 +975,7 @@ main(int argc, char **argv)
> + 			if (bitmap.maxcorrect)
> + 				fprintf(stderr,
> + 					"Correctable message size: %d bits, %0.2f%%\n",
> +-					bitmap.maxcorrect,
> ++					(int)bitmap.maxcorrect,
> + 					(float)100*bitmap.maxcorrect/bitmap.bits);
> + 		}
> +
> +--
> +2.9.3
> +
> diff --git a/app-crypt/outguess/files/outguess-0.2-fix-segfault-when-encoding-PNM-images.patch b/app-crypt/outguess/files/outguess-0.2-fix-segfault-when-encoding-PNM-images.patch
> new file mode 100644
> index 00000000000..40c44a21bd6
> --- /dev/null
> +++ b/app-crypt/outguess/files/outguess-0.2-fix-segfault-when-encoding-PNM-images.patch
> @@ -0,0 +1,29 @@
> +From 78096e8fb29f7b639b913472f089f90f9bf21ed2 Mon Sep 17 00:00:00 2001
> +From: Chris Rorvick <chris@×××××××.com>
> +Date: Thu, 1 Sep 2016 00:43:06 -0500
> +Subject: [PATCH] fix segfault when encoding PNM images
> +
> +OutGuess 0.2 added the ability to preserve frequency counts in JPEG
> +images to foil statistical tests.  This was stubbed out in the PNM
> +driver but forgot to do some necessary setup.  Assign `bitmap.preserve'
> +accordingly to avoid calling a null pointer.
> +---
> + pnm.c | 2 ++
> + 1 file changed, 2 insertions(+)
> +
> +diff --git a/pnm.c b/pnm.c
> +index 3bfc330..f1aa65d 100644
> +--- a/pnm.c
> ++++ b/pnm.c
> +@@ -63,6 +63,8 @@ init_pnm(char *parameter)
> + int
> + preserve_pnm(bitmap *bitmap, int off)
> + {
> ++	if (off == -1)
> ++		bitmap->preserve = preserve_pnm;
> +
> + 	return (-1);

Is it really supposed to return -1 in this case? The whole function
looks quite weird to me.

> + }
> +--
> +2.9.3
> +
> diff --git a/app-crypt/outguess/files/outguess-0.2-pass-struct-size-to-memset.patch b/app-crypt/outguess/files/outguess-0.2-pass-struct-size-to-memset.patch
> new file mode 100644
> index 00000000000..f1b1458d428
> --- /dev/null
> +++ b/app-crypt/outguess/files/outguess-0.2-pass-struct-size-to-memset.patch
> @@ -0,0 +1,25 @@
> +From 37a4bf9523f18e5e1b064becf50112d47fae2acd Mon Sep 17 00:00:00 2001
> +From: Chris Rorvick <chris@×××××××.com>
> +Date: Thu, 1 Sep 2016 01:49:14 -0500
> +Subject: [PATCH] pass struct size to memset(), not pointer size
> +
> +---
> + missing/md5.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/missing/md5.c b/missing/md5.c
> +index 314548a..2da3835 100644
> +--- a/missing/md5.c
> ++++ b/missing/md5.c
> +@@ -168,7 +168,7 @@ MD5Final(digest, ctx)
> + 	putu32(ctx->buf[1], digest + 4);
> + 	putu32(ctx->buf[2], digest + 8);
> + 	putu32(ctx->buf[3], digest + 12);
> +-	memset(ctx, 0, sizeof(ctx));	/* In case it's sensitive */
> ++	memset(ctx, 0, sizeof(*ctx));	/* In case it's sensitive */
> + }
> +
> + #ifndef ASM_MD5
> +--
> +2.9.3
> +
> diff --git a/app-crypt/outguess/metadata.xml b/app-crypt/outguess/metadata.xml
> new file mode 100644
> index 00000000000..c47298e10ff
> --- /dev/null
> +++ b/app-crypt/outguess/metadata.xml
> @@ -0,0 +1,19 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!DOCTYPE pkgmetadata SYSTEM "http://www.gentoo.org/dtd/metadata.dtd">;
> +<pkgmetadata>
> +  <maintainer type="person">
> +    <email>chris@×××××××.com

It seems that there is no Gentoo Bugzilla address registered with this
e-mail address. This field is used to assign bugs, so please either
register one or adjust the e-mails to match your account.

> +    <name>Chris Rorvick</name>
> +  </maintainer>

You need to include proxy-maint team here to commit for you.

> +  <longdescription lang="en">
> +    OutGuess is a universal steganographic tool that allows the
> +	insertion of hidden information into the redundant bits of data
> +	sources.  The nature of the data source is irrelevant to the core of
> +	OutGuess.  The program relies on data specific handlers that will
> +	extract redundant bits and write them back after modification.  In
> +	this version the PNM and JPEG image formats are supported.  In the
> +	next paragraphs, images will be used as concrete example of data
> +	objects, though OutGuess can use any kind of data, as long as a
> +	handler is provided.
> +  </longdescription>
> +</pkgmetadata>
> diff --git a/app-crypt/outguess/outguess-0.2-r1.ebuild b/app-crypt/outguess/outguess-0.2-r1.ebuild
> new file mode 100644
> index 00000000000..69d882b2ad3
> --- /dev/null
> +++ b/app-crypt/outguess/outguess-0.2-r1.ebuild
> @@ -0,0 +1,27 @@
> +# Copyright 1999-2017 Gentoo Foundation
> +# Distributed under the terms of the GNU General Public License v2
> +
> +EAPI=6
> +
> +DESCRIPTION="A universal tool for inserting steganographic information into other data"
> +HOMEPAGE="http://www.outguess.org/"

This homepage doesn't seem to work.

> +SRC_URI="https://dl.packetstormsecurity.net/crypt/stego/outguess-0.2.tar.gz"

Please use ${P}.tar.gz to avoid having to update the URL every time
version changes. Even if you don't predict it ever happening.

> +
> +LICENSE="BSD"
> +SLOT="0"
> +KEYWORDS="~amd64 ~x86"
> +IUSE=""
> +
> +PATCHES=(
> +	"${FILESDIR}"/${P}-fix-segfault-when-encoding-PNM-images.patch
> +	"${FILESDIR}"/${P}-pass-struct-size-to-memset.patch
> +	"${FILESDIR}"/${P}-avoid-implicit-function-usage.patch
> +	"${FILESDIR}"/${P}-fix-printf-argument-types.patch
> +)
> +
> +S=${WORKDIR}/${PN}
> +
> +src_install() {
> +	dobin outguess
> +	doman outguess.1
> +}

To be honest, I have mixed feelings about this package. It seems to be
completely dead and requires a lot of patches. I see you have a GitHub
fork of it though.

If you're really interested in it, then I think it'd be reasonable if
you just packaged your own fork/continuation, i.e. made a new fixed
release instead of including all the patches inline. Of course, this
also assumes you're willing to fix any bugs there might arise
in the future.

Otherwise, I don't really see us including a package that's been
abandoned in 2001 and has a lot of known issues.

-- 
Best regards,
Michał Górny