You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2014/02/21 04:06:39 UTC
svn commit: r1570434 - /subversion/trunk/subversion/svnserve/cyrus_auth.c
Author: breser
Date: Fri Feb 21 03:06:39 2014
New Revision: 1570434
URL: http://svn.apache.org/r1570434
Log:
Fix potential integer overflow in Cyrus SASL support for svnserve.
* subversion/svnserve/cyrus_auth.c
(try_auth): error if we'll overflow the clientinlen argument to sasl
functions, typecast to silence the compiler warning.
Modified:
subversion/trunk/subversion/svnserve/cyrus_auth.c
Modified: subversion/trunk/subversion/svnserve/cyrus_auth.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/cyrus_auth.c?rev=1570434&r1=1570433&r2=1570434&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/cyrus_auth.c (original)
+++ subversion/trunk/subversion/svnserve/cyrus_auth.c Fri Feb 21 03:06:39 2014
@@ -184,9 +184,15 @@ static svn_error_t *try_auth(svn_ra_svn_
/* For CRAM-MD5, we don't base64-encode stuff. */
use_base64 = (strcmp(mech, "CRAM-MD5") != 0);
+ /* sasl uses unsigned for the length of strings, we use apr_size_t
+ * which may not be the same size. Deal with potential integer overflow */
+ if (in->len > UINT_MAX)
+ return svn_error_createf(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
+ _("Initial token is too long"));
+
result = sasl_server_start(sasl_ctx, mech,
in ? in->data : NULL,
- in ? in->len : 0, &out, &outlen);
+ in ? (unsigned) in->len : 0, &out, &outlen);
if (result != SASL_OK && result != SASL_CONTINUE)
return fail_auth(conn, pool, sasl_ctx);
@@ -210,7 +216,13 @@ static svn_error_t *try_auth(svn_ra_svn_
in = item->u.string;
if (use_base64)
in = svn_base64_decode_string(in, pool);
- result = sasl_server_step(sasl_ctx, in->data, in->len, &out, &outlen);
+
+ if (in->len > UINT_MAX)
+ return svn_error_createf(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
+ _("Step response is too long"));
+
+ result = sasl_server_step(sasl_ctx, in->data, (unsigned) in->len,
+ &out, &outlen);
}
if (result != SASL_OK)
Re: svn commit: r1570434 - /subversion/trunk/subversion/svnserve/cyrus_auth.c
Posted by Ben Reser <be...@reser.org>.
On 2/21/14, 1:43 AM, Stefan Sperling wrote:
> Nice fix. Can we please write 'unsigned int'? I know there is a
> default size, which is probably int, but I already have to remember
> so many other idiosyncrasies about C...
Done in r1570648. I used just unsigned since I just used what Cyrus SASL had in
their API.
Re: svn commit: r1570434 -
/subversion/trunk/subversion/svnserve/cyrus_auth.c
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Feb 21, 2014 at 03:06:39AM -0000, breser@apache.org wrote:
> Author: breser
> Date: Fri Feb 21 03:06:39 2014
> New Revision: 1570434
>
> URL: http://svn.apache.org/r1570434
> Log:
> Fix potential integer overflow in Cyrus SASL support for svnserve.
>
> * subversion/svnserve/cyrus_auth.c
> (try_auth): error if we'll overflow the clientinlen argument to sasl
> functions, typecast to silence the compiler warning.
>
> Modified:
> subversion/trunk/subversion/svnserve/cyrus_auth.c
> - result = sasl_server_step(sasl_ctx, in->data, in->len, &out, &outlen);
> +
> + if (in->len > UINT_MAX)
> + return svn_error_createf(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
> + _("Step response is too long"));
> +
> + result = sasl_server_step(sasl_ctx, in->data, (unsigned) in->len,
> + &out, &outlen);
Nice fix. Can we please write 'unsigned int'? I know there is a
default size, which is probably int, but I already have to remember
so many other idiosyncrasies about C...