Date: Fri, 13 Nov 2020 09:44:23 -0500 From: Greg Veldman <freebsd@gregv.net> To: Yuri Pankov <ypankov@xsmail.com> Cc: Andrea Venturoli <ml@netfence.it>, freebsd-ports@freebsd.org, freebsd@gregv.net Subject: Re: Sudden trouble with net/rdesktop Message-ID: <20201113144423.GK1045@aurora.gregv.net> In-Reply-To: <a8c01ca0-7d4c-7d8a-e07c-b328ac0b9e20@xsmail.com> References: <1420575d-2622-2063-3a2f-bb0591b9f71c@netfence.it> <0af296a2-2896-87ba-3f8e-de68276a3baf@xsmail.com> <e7783ede-e860-8498-6acc-3f5b15d58e89@xsmail.com> <8d25cba0-63ca-076e-2340-22e31a3015d5@netfence.it> <a8c01ca0-7d4c-7d8a-e07c-b328ac0b9e20@xsmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Nov 13, 2020 at 03:52:54PM +0300, Yuri Pankov wrote:
> OK, I was able to reproduce this; actually, it hit that assertion for
> all Windows 10 20H2 installs I have, all are updated to latest patches,
> so I can't tell when this started to happen. Adding a debug print
> before that assert() shows the following:
>
> $ DISPLAY=mercury:0.0
> /usr/ports/net/rdesktop/work/rdesktop-1.9.0/rdesktop orion
> len=128 size=128
> Assertion failed: ((len * 2) < size), function _utils_data_to_hex, file
> utils.c, line 501.
>
> So we need the len of 128 we need size of digest buf to be > 256, the
> following patch worked for me:
>
> polaris:ypankov:/usr/ports/net/rdesktop$ cat files/patch-utils.c
> --- utils.c.orig 2020-11-13 12:40:51 UTC
> +++ utils.c
> @@ -584,7 +584,7 @@ _utils_cert_get_info(gnutls_x509_crt_t cert, char *out
> {
> char buf[128];
> size_t buf_size;
> - char digest[128];
> + char digest[512];
> gnutls_x509_dn_t dn;
> time_t expire_ts, activated_ts;
This seems like a reasonable change, but I'll make one minor
note. Later on in _utils_cert_get_info() the contents of
digest are written to sha1 and sha256, which are both size
256. In the initial implementation both buf and digest are
of the same size, which would seem to indicate that buf was
not expected to be completely full (in fact less than half
full based on the assertion in _utils_data_to_hex()). However
if defined to 512 chars and ever somehow filled beyond 256
(in fact a bit less than that, since a static string is
prepended to the call to snprintf()), the writes to sha1 and
sha256 would be incomplete, which I haven't completely read
through but would very likely lead to Bad Things(tm) later on.
All this to say, perhaps a more conservative approach would be
to increase digest to 256 chars instead of 512. I don't currently
have a Windows 10 20H2 machine to test on, but would you mind
trying with 256 instead of 512 and see if that also fixes the
issue? If so, let me know and I'll submit a PR to get this
patch added.
Thanks.
--
Greg Veldman
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20201113144423.GK1045>