Re: AIX 5.1 client crash (buffer-overflow)

Travis <svn@castle.fastmail.fm> writes:

> Yes, xlc, version 6 on AIX 5.1.
>
> I had to make these changes to successfully compile because they
> stopped compilation:
>
> "subversion/libsvn_ra_svn/cram.c", line 90.13: 1506-280 (E) Function
> argument
> assignment between types "unsigned char*" and "char*" is not allowed.
> "subversion/libsvn_ra_svn/cram.c", line 99.17: 1506-280 (E) Function
> argument
> assignment between types "unsigned char*" and "char*" is not allowed.
> "subversion/libsvn_ra_svn/cram.c", line 105.17: 1506-280 (E) Function
> argument
> assignment between types "unsigned char*" and "char*" is not allowed.
>
> % diff orig/subversion-0.35.1/subversion/libsvn_ra_svn/cram.c \
> subversion-0.35.1/subversion/libsvn_ra_svn/cram.c
> 90c90
> < apr_md5(secret, password, len);
> ---
> > apr_md5((unsigned char*)secret, password, len);
> 99c99
> < apr_md5_final(digest, &ctx);
> ---
> > apr_md5_final((unsigned char*)digest, &ctx);
> 105c105
> < apr_md5_final(digest, &ctx);
> ---
> > apr_md5_final((unsigned char*)digest, &ctx);

Hmm, that function does bit operations on char, and char could be
signed. Perhaps it would be better to use unsigned char. Greg?

Index: subversion/libsvn_ra_svn/cram.c
===================================================================
--- subversion/libsvn_ra_svn/cram.c (revision 8340)
+++ subversion/libsvn_ra_svn/cram.c (working copy)
@@ -78,7 +78,7 @@
 static void compute_digest(char *digest, const char *challenge,
                            const char *password)
 {
- char secret[64];
+ unsigned char secret[64];
   apr_size_t len = strlen(password), i;
   apr_md5_ctx_t ctx;
 

> "subversion/clients/cmdline/main.c", line 611.17: 1506-112 (E)
> Duplicate type
> qualifier "volatile" ignored.
>
> % diff orig/subversion-0.35.1/subversion/clients/cmdline/main.c \
> subversion-0.35.1/subversion/clients/cmdline/main.c
> 611c611
> < static volatile sig_atomic_t cancelled = FALSE;
> ---
> > static sig_atomic_t cancelled = FALSE;

Hmm, again. I think this is a real AIX bug. The C standard is
explicit, we need to use "volatile sig_atomic_t". I see that gcc's
fixincl removes volatile from the shadow sys/signal.h, but says that
only some versions of AIX are affected. We need to be very careful
not to remove volatile from our code in cases where there is no
duplicate. Perhaps we could use an autoconf macro. Or we could use
an integer type directly, but that's no more portable than not using
volatile. Or we could leave it as it is, but add a comment to the
code saying that if the error occurs then volatile should be removed.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org