On Mon, Aug 05, 2024 at 10:13:16AM -0700, Russ Allbery wrote:
> Niko Tyni <nt...@debian.org> writes:

> > If it's really the right thing to do, I suppose casting to an IV would
> > be more correct than either int or long int.  But I suspect that the
> > intention might be to dereference the pointer instead. FWIW I tried that
> > at home and the test suite still passed, so clearly it's not covering
> > these parts.
> 
> I agree.  I think the upstream code is buggy here and was incorrectly
> returning the value of the pointer rather than the underlying property
> value.

Thanks for confirming!

I propose the attached patch on top of Étienne's. My understanding of
for example https://www.sendmail.org/~ca/email/cyrus2/mechanisms.html
is that the security strength factor (SSF) is always zero for PLAIN
connections, so I'm testing for that value. I've checked that the new
test fails as expected (yielding a pointer value as a large integer)
without the code change.

> > I don't see any reverse dependencies, so removal is also an option.
> > Particularly as this seems security sensitive and abandoned upstream...
> 
> The lack of dependencies is somewhat deceptive because this module is
> transparently used by Authen::SASL (which is, somewhat surprisingly,
> missing any relevant dependency, even at the Suggests level; that's
> probably a different bug).  I believe it prefers Authen::SASL::XS if it is
> installed.
> 
> The Perl implementation for Authen::SASL works fine for clients, but if
> you want to write a server, you need Authen::SASL::XS if you're using any
> mechanism other than the simple password ones.  See Authen::SASL::Perl:
> 
>     As for server support, only *PLAIN*, *LOGIN* and *DIGEST-MD5* are
>     supported at the time of this writing.

Okay, let's keep it then :) Thanks again.
-- 
Niko
From: Niko Tyni <nt...@debian.org>
Date: Mon, 5 Aug 2024 16:50:56 +0100
X-Dgit-Generated: 1.00-2 fe76997d50267530dd5a5f73995d11987547ac4d
Subject: Fix SASL_SSF and SASL_MAXOUTBUF property handling

sasl_getprop() returns a pointer which needs to be dereferenced to get
the actual value.

Bug-Debian: https://bugs.debian.org/1075146

---

diff --git a/XS.xs b/XS.xs
index eaeef7e..d6d6f4b 100755
--- a/XS.xs
+++ b/XS.xs
@@ -1883,7 +1883,7 @@ PPCODE:
 			break;
 			case SASL_SSF:
 			case SASL_MAXOUTBUF:
-				XPUSHi((long int)value);
+				XPUSHi(*((IV *)value));
 			break;
 #ifdef SASL2
 			case SASL_IPLOCALPORT:
diff --git a/t/plain.t b/t/plain.t
index 6776cf6..4730905 100755
--- a/t/plain.t
+++ b/t/plain.t
@@ -1,7 +1,7 @@
 
 use strict;
 use Authen::SASL qw(XS); # Only use XS plugin
-use Test::Simple tests => 5;
+use Test::Simple tests => 6;
 
 our $me;
 require "t/common.pl";
@@ -38,6 +38,9 @@ sleep(1);
 
 	print $conn->listmech("","|",""),"\n";
 
+	my $ssf = $conn->property('ssf');
+	ok($ssf == 0, "security strength factor $ssf is zero as expected");
+
 	sendreply( $conn->server_start( getreply(\*FROM_CLIENT)  ),\*TO_CLIENT );
 	ok(1);
 	

Reply via email to