From 4b6f8dcd2a838f0528a4de1d9c247101fe7907fb Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Fri, 21 Dec 2012 16:13:03 +0000 Subject: [PATCH] Core: crypt_r() error handling fixed. The crypt_r() function returns NULL on errors, check it explicitly instead of assuming errno will remain 0 if there are no errors (per POSIX, the setting of errno after a successful call to a function is unspecified unless the description of that function specifies that errno shall not be modified). Additionally, dropped unneeded ngx_set_errno(0) and fixed error handling of memory allocation after normal crypt(), which was inapropriate and resulted in null pointer dereference on allocation failures. --- src/os/unix/ngx_user.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/os/unix/ngx_user.c b/src/os/unix/ngx_user.c index 27f990e83..7a71203cb 100644 --- a/src/os/unix/ngx_user.c +++ b/src/os/unix/ngx_user.c @@ -28,30 +28,27 @@ ngx_libc_crypt(ngx_pool_t *pool, u_char *key, u_char *salt, u_char **encrypted) { char *value; size_t len; - ngx_err_t err; struct crypt_data cd; - ngx_set_errno(0); - cd.initialized = 0; /* work around the glibc bug */ cd.current_salt[0] = ~salt[0]; value = crypt_r((char *) key, (char *) salt, &cd); - err = ngx_errno; - - if (err == 0) { + if (value) { len = ngx_strlen(value) + 1; *encrypted = ngx_pnalloc(pool, len); - if (*encrypted) { - ngx_memcpy(*encrypted, value, len); - return NGX_OK; + if (*encrypted == NULL) { + return NGX_ERROR; } + + ngx_memcpy(*encrypted, value, len); + return NGX_OK; } - ngx_log_error(NGX_LOG_CRIT, pool->log, err, "crypt_r() failed"); + ngx_log_error(NGX_LOG_CRIT, pool->log, ngx_errno, "crypt_r() failed"); return NGX_ERROR; } @@ -75,18 +72,20 @@ ngx_libc_crypt(ngx_pool_t *pool, u_char *key, u_char *salt, u_char **encrypted) #endif - ngx_set_errno(0); - value = crypt((char *) key, (char *) salt); if (value) { len = ngx_strlen(value) + 1; *encrypted = ngx_pnalloc(pool, len); - if (*encrypted) { - ngx_memcpy(*encrypted, value, len); + if (*encrypted == NULL) { +#if (NGX_THREADS && NGX_NONREENTRANT_CRYPT) + ngx_mutex_unlock(ngx_crypt_mutex); +#endif + return NGX_ERROR; } + ngx_memcpy(*encrypted, value, len); #if (NGX_THREADS && NGX_NONREENTRANT_CRYPT) ngx_mutex_unlock(ngx_crypt_mutex); #endif