[Date Prev][Date Next] [Chronological] [Thread] [Top]

(ITS#6079) TLS error strings need to be printf() safe



Full_Name: Aaron Richton
Version: 2.4.16
OS: Solaris 9
URL: 
Submission from: (NULL) (128.6.31.135)


Personally I focus on OpenSSL, where ERR_error_string is permitted to return
NULL, so we can't Debug() them arbitrarily. This is handled in _connect:

 390                 Debug( LDAP_DEBUG_ANY,"TLS: can't connect: %s.\n",
 391                         ld->ld_error ? ld->ld_error : "" ,0,0);

but not in _accept:

 440                 Debug( LDAP_DEBUG_ANY,"TLS: can't accept: %s.\n",
 441                         tls_imp->ti_session_errmsg( err, buf, sizeof(buf)
),0,0 );

and at least some rc's don't have error strings, so it's all too easy to:

t@14 (l@14) signal SEGV (no mapping at the fault address) in strlen at
0xffffffff7ee3d50c
0xffffffff7ee3d50c: strlen+0x007c:	ld	 [%o1], %o2
Current function is ldap_log_printf
current thread: t@14
  [1] strlen(0x0, 0x0, 0xffffffff46bff1d0, 0x7efefeff, 0x81010100, 0x107a59610),
at 0xffffffff7ee3d50c
  [2] _doprnt(0xffffffff7ea78960, 0xffffffff46bff1d0, 0xffffffff46bff1d0, 0x0,
0x73, 0x0), at 0xffffffff7ee90f50
  [3] vsnprintf(0xffffffff46bff31c, 0x3ff, 0xffffffff7ea78960,
0xffffffff46bff7d8, 0x0, 0xffffffff79a23e48), at 0xffffffff7ee9310c
=>[4] ldap_log_printf(ld = (nil), loglvl = -1, fmt = 0xffffffff7ea78960 "TLS:
can't accept: %s.\n", ... = (nil), ...), line 56 in "print.c"
  [5] ldap_pvt_tls_accept(sb = 0x11d459e90, ctx_arg = 0x100a6c610), line 441 in
"tls2.c"
  [6] connection_read(s = 215, cri = 0xffffffff46bffaf8), line 1317 in
"connection.c"
  [7] connection_read_thread(ctx = 0xffffffff46bffc20, argv = 0xd7), line 1216
in "connection.c"
  [8] ldap_int_thread_pool_wrapper(xpool = 0x10064f6a0), line 663 in "tpool.c"


I don't know if you want to do a similar tls_imp->() ? tls_imp->() : "" (it'd
work but it looks ugly) or make this the job of *_session_errmsg so I didn't
write a patch for this...sorry. Actually, come to think of it, there is:

 407         return NULL;

in tlso_session_errmsg, that would be dangerous too. NULL might also be returned
in tls_m. I don't know is gnutls_strerror is safe or not...