tidyin

plugins/auth/src/pam_password.cpp

@@ -374,9 +374,10 @@ namespace irods
 
             log_auth::trace("PAM auth check done", username);
 
-            char password_out[MAX_NAME_LEN]{};
-            char* pw_ptr = &password_out[0];
-            const int ec = chlUpdateIrodsPamPassword(&comm, const_cast<char*>(username.c_str()), ttl, nullptr, &pw_ptr);
+            std::array<char, MAX_NAME_LEN> password_out{};
+            char* password_ptr = password_out.data();
+            const int ec = chlUpdateIrodsPamPassword(
+                &comm, const_cast<char*>(username.c_str()), ttl, nullptr, &password_ptr, MAX_NAME_LEN);
             if (ec < 0) {
                 THROW(ec, "failed updating iRODS pam password");
             }

plugins/auth_legacy/src/pam.cpp

@@ -404,16 +404,17 @@ irods::error pam_auth_agent_request(
 
     // =-=-=-=-=-=-=-
     // request the resulting irods password after the handshake
-    char password_out[ MAX_NAME_LEN ];
-    char* pw_ptr = &password_out[0];
-    status = chlUpdateIrodsPamPassword( _ctx.comm(), const_cast< char* >( user_name.c_str() ), ttl, NULL, &pw_ptr );
+    std::array<char, MAX_NAME_LEN> password_out{};
+    char* password_ptr = password_out.data();
+    status = chlUpdateIrodsPamPassword(
+        _ctx.comm(), const_cast<char*>(user_name.c_str()), ttl, nullptr, &password_ptr, MAX_NAME_LEN);
     if (status < 0) {
         return ERROR(status, "failed updating iRODS pam password");
     }
 
     // =-=-=-=-=-=-=-
     // set the result for communication back to the client
-    ptr->request_result( password_out );
+    ptr->request_result(password_out.data());
 
     // =-=-=-=-=-=-=-
     // win!

plugins/database/src/db_plugin.cpp

@@ -7064,27 +7064,32 @@ irods::error db_make_limited_pw_op(
 
 // =-=-=-=-=-=-=-
 // authenticate user
-irods::error db_update_pam_password_op(
-    irods::plugin_context& _ctx,
-    const char*            _user_name,
-    int                    _ttl,
-    const char*            _test_time,
-    char**                 _irods_password ) {
+auto db_update_pam_password_op(irods::plugin_context& _ctx,
+                               const char* _user_name,
+                               int _ttl,
+                               const char* _test_time,
+                               char** _password_buffer,
+                               std::size_t _password_buffer_size) -> irods::error
+{
     // =-=-=-=-=-=-=-
     // check the context
     irods::error ret = _ctx.valid();
     if ( !ret.ok() ) {
         return PASS( ret );
     }
 
-    // =-=-=-=-=-=-=-
-    // check the params
-    if (
-        !_user_name ||
-        !_irods_password ) {
+    if (!_user_name || !_password_buffer) {
+        return ERROR(CAT_INVALID_ARGUMENT, "null parameter");
+    }
+
+    std::array<char, MAX_PASSWORD_LEN> random_password{};
+    if (random_password.size() > _password_buffer_size) {
         return ERROR(
-                   CAT_INVALID_ARGUMENT,
-                   "null parameter" );
+            SYS_INVALID_INPUT_PARAM,
+            fmt::format("{}: Buffer not large enough to hold password. Requires [{}] bytes, received [{}] bytes.",
+                        __func__,
+                        random_password.size(),
+                        _password_buffer_size));
     }
 
     // =-=-=-=-=-=-=-
@@ -7177,8 +7182,7 @@ irods::error db_update_pam_password_op(
         log_sql::debug("chlUpdateIrodsPamPassword SQL 3");
     }
 
-    std::array<char, MAX_PASSWORD_LEN + 2> stored_random_password{};
-    cVal[0] = stored_random_password.data();
+    cVal[0] = random_password.data();
     iVal[0] = MAX_PASSWORD_LEN;
     cVal[1] = passwordModifyTime;
     iVal[1] = sizeof( passwordModifyTime );
@@ -7205,7 +7209,7 @@ irods::error db_update_pam_password_op(
             cllBindVars[cllBindVarCount++] = expTime;
             cllBindVars[cllBindVarCount++] = selUserId;
             // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index)
-            cllBindVars[cllBindVarCount++] = stored_random_password.data();
+            cllBindVars[cllBindVarCount++] = random_password.data();
             status =  cmlExecuteNoAnswerSql( "update R_USER_PASSWORD set modify_ts=?, pass_expiry_ts=? where user_id = ? and rcat_password = ?",
                                              &icss );
             if ( status ) {
@@ -7219,16 +7223,15 @@ irods::error db_update_pam_password_op(
             }
         } // if !irods_pam_auth_no_extend
 
-        // stored_random_password is the randomly generated password (see while loop below) in a scrambled form.
-        icatDescramble(stored_random_password.data());
+        // random_password is the randomly generated password (see while loop below) in a scrambled form.
+        icatDescramble(random_password.data());
 
         // The descrambled password at time of generation is 50 characters or less (see below).
-        std::strncpy(*_irods_password, stored_random_password.data(), stored_random_password.size());
+        std::strncpy(*_password_buffer, random_password.data(), _password_buffer_size);
 
         return SUCCESS();
     }
 
-    std::array<char, MAX_PASSWORD_LEN> random_password{};
     std::array<char, MAX_PASSWORD_LEN> scrambled_random_password{};
 
     constexpr auto random_bytes_count = 64 + 1; // +1 for null terminator
@@ -7276,9 +7279,9 @@ irods::error db_update_pam_password_op(
         return ERROR( status, "commit failure" );
     }
 
-    std::strncpy(*_irods_password, random_password.data(), random_password.size());
-    return SUCCESS();
+    std::strncpy(*_password_buffer, random_password.data(), _password_buffer_size);
 
+    return SUCCESS();
 } // db_update_pam_password_op
 
 // =-=-=-=-=-=-=-
@@ -15842,10 +15845,9 @@ irods::database* plugin_factory(
         DATABASE_OP_MAKE_TEMP_PW,
         function<error(plugin_context&,char*, const char*)>(
             db_make_temp_pw_op ) );
-    pg->add_operation(
-        DATABASE_OP_UPDATE_PAM_PASSWORD,
-        function<error(plugin_context&,const char*,int,const char*,char**)>(
-            db_update_pam_password_op ) );
+    pg->add_operation(DATABASE_OP_UPDATE_PAM_PASSWORD,
+                      function<error(plugin_context&, const char*, int, const char*, char**, std::size_t)>(
+                          db_update_pam_password_op));
     pg->add_operation(
         DATABASE_OP_MOD_USER,
         function<error(plugin_context&,const char*,const char*,const char*)>(

server/api/src/rsPamAuthRequest.cpp

@@ -1,20 +1,16 @@
-/*** Copyright (c), The Regents of the University of California            ***
- *** For more information please refer to files in the COPYRIGHT directory ***/
-
-/* See pamAuthRequest.h for a description of this API call.*/
-
-#include <sys/wait.h>
+#include "irods/rsPamAuthRequest.hpp"
 
-#include "irods/pamAuthRequest.h"
+#include "irods/authenticate.h"
 #include "irods/genQuery.h"
-#include "irods/rsPamAuthRequest.hpp"
 #include "irods/icatHighLevelRoutines.hpp"
-#include "irods/miscServerFunct.hpp"
-#include "irods/irods_server_properties.hpp"
 #include "irods/irods_log.hpp"
-#include "irods/sslSockComm.h"
+#include "irods/irods_server_properties.hpp"
+#include "irods/miscServerFunct.hpp"
 #include "irods/miscServerFunct.hpp"
+#include "irods/pamAuthRequest.h"
+#include "irods/sslSockComm.h"
 
+#include <sys/wait.h>
 
 int
 rsPamAuthRequest( rsComm_t *rsComm, pamAuthRequestInp_t *pamAuthRequestInp,
@@ -127,14 +123,11 @@ int
 _rsPamAuthRequest( rsComm_t *rsComm, pamAuthRequestInp_t *pamAuthRequestInp,
                    pamAuthRequestOut_t **pamAuthRequestOut ) {
     int status = 0;
-    pamAuthRequestOut_t *result;
 
     *pamAuthRequestOut = ( pamAuthRequestOut_t * )
                          malloc( sizeof( pamAuthRequestOut_t ) );
     memset( ( char * )*pamAuthRequestOut, 0, sizeof( pamAuthRequestOut_t ) );
 
-    result = *pamAuthRequestOut;
-
     /* Normal mode, fork/exec setuid program to do the Pam check */
     status = runPamAuthCheck( pamAuthRequestInp->pamUser,
                               pamAuthRequestInp->pamPassword );
@@ -151,15 +144,21 @@ _rsPamAuthRequest( rsComm_t *rsComm, pamAuthRequestInp_t *pamAuthRequestInp,
     if ( status ) {
         return status;
     }
-    result->irodsPamPassword = ( char* )malloc( 100 );
-    if ( result->irodsPamPassword == 0 ) {
+
+    pamAuthRequestOut_t* result = *pamAuthRequestOut;
+
+    // NOLINTNEXTLINE(cppcoreguidelines-owning-memory, cppcoreguidelines-no-malloc)
+    result->irodsPamPassword = static_cast<char*>(std::malloc(MAX_PASSWORD_LEN));
+    if (nullptr == result->irodsPamPassword) {
         return SYS_MALLOC_ERR;
     }
-    memset(result->irodsPamPassword, 0, 100);
-    status = chlUpdateIrodsPamPassword( rsComm,
-                                        pamAuthRequestInp->pamUser,
-                                        pamAuthRequestInp->timeToLive,
-                                        NULL,
-                                        &result->irodsPamPassword );
+
+    std::memset(result->irodsPamPassword, 0, MAX_PASSWORD_LEN);
+    status = chlUpdateIrodsPamPassword(rsComm,
+                                       pamAuthRequestInp->pamUser,
+                                       pamAuthRequestInp->timeToLive,
+                                       nullptr,
+                                       &result->irodsPamPassword,
+                                       MAX_PASSWORD_LEN);
     return status;
 }

server/icat/include/irods/icatHighLevelRoutines.hpp

@@ -194,9 +194,12 @@ int chlVersionFnmBase( rsComm_t *rsComm,
 int chlModTicket( rsComm_t *rsComm, const char *opName, const char *ticket,
                   const char *arg1, const char *arg2, const char *arg3,
                   const KeyValPair *condInput);
-int chlUpdateIrodsPamPassword( rsComm_t *rsComm, const char *userName,
-                               int timeToLive, const char *testTime,
-                               char **irodsPassword );
+auto chlUpdateIrodsPamPassword(rsComm_t* rsComm,
+                               const char* userName,
+                               int timeToLive,
+                               const char* testTime,
+                               char** _password_buffer,
+                               std::size_t _password_buffer_size) -> int;
 
 /// =-=-=-=-=-=-=-
 /// @brief typedefs and prototype for query used for rebalancing operation

server/icat/src/icatHighLevelRoutines.cpp

@@ -2038,12 +2038,13 @@ chlMakeLimitedPw(
 // If one already exists, the expire time is updated, and it's value is returned.
 // Passwords created are pseudo-random strings, unrelated to the PAM password.
 // If testTime is non-null, use that as the create-time, as a testing aid.
-int chlUpdateIrodsPamPassword(
-    rsComm_t* _comm,
-    const char*     _user_name,
-    int             _ttl,
-    const char*     _test_time,
-    char**    _irods_password ) {
+auto chlUpdateIrodsPamPassword(rsComm_t* _comm,
+                               const char* _user_name,
+                               int _ttl,
+                               const char* _test_time,
+                               char** _password_buffer,
+                               std::size_t _password_buffer_size) -> int
+{
     // =-=-=-=-=-=-=-
     // call factory for database object
     irods::database_object_ptr db_obj_ptr;
@@ -2078,18 +2079,14 @@ int chlUpdateIrodsPamPassword(
 
     // =-=-=-=-=-=-=-
     // call the operation on the plugin
-    ret = db->call <
-          const char*,
-          int,
-          const char*,
-          char** > (
-              _comm,
-              irods::DATABASE_OP_UPDATE_PAM_PASSWORD,
-              ptr,
-              _user_name,
-              _ttl,
-              _test_time,
-              _irods_password );
+    ret = db->call(_comm,
+                   irods::DATABASE_OP_UPDATE_PAM_PASSWORD,
+                   ptr,
+                   _user_name,
+                   _ttl,
+                   _test_time,
+                   _password_buffer,
+                   _password_buffer_size);
 
     return ret.code();