otp: Copy strings with explicit limiting

When copying username, challenge and password in otp_pw_valid, use
strlcpy accepting explicit destination size and verify its result,
instead of first assuming or verifying the string will fit and then
doing unlimited strcpy.

This silences the following Coverity errors.

Error: STRING_OVERFLOW (CWE-120):
freeradius-server-3.0.4rc2/src/modules/rlm_otp/otp_pw_valid.c:89: fixed_size_dest: You might overrun the 32 byte fixed-size string "otp_request.username" by copying "username" without checking the length.

Error: STRING_OVERFLOW (CWE-120):
freeradius-server-3.0.4rc2/src/modules/rlm_otp/otp_pw_valid.c:90: fixed_size_dest: You might overrun the 17 byte fixed-size string "otp_request.challenge" by copying "challenge" without checking the length.
freeradius-server-3.0.4rc2/src/modules/rlm_otp/otp_pw_valid.c:90: parameter_as_source: Note: This defect has an elevated risk because the source argument is a parameter of the current function.

Error: STRING_OVERFLOW (CWE-120):
freeradius-server-3.0.4rc2/src/modules/rlm_otp/otp_pw_valid.c:122: fixed_size_dest: You might overrun the 48 byte fixed-size string "otp_request.pwe.u.pap.passcode" by copying "rvp->data.strvalue" without checking the length.

diff --git a/src/modules/rlm_otp/otp_pw_valid.c b/src/modules/rlm_otp/otp_pw_valid.c
index 54ec500620caf2d19f9d99943bd3558789dcd632..4e862095361457df4c4914031a1cf01c0a4f39bd 100644 (file)
--- a/src/modules/rlm_otp/otp_pw_valid.c
+++ b/src/modules/rlm_otp/otp_pw_valid.c
@@ -78,16 +78,20 @@ int otp_pw_valid(REQUEST *request, int pwe, char const *challenge,
        char const      *username = request->username->vp_strvalue;
        int             rc;
 
-       if (request->username->length > OTP_MAX_USERNAME_LEN) {
+       otp_request.version = 2;
+
+       if (strlcpy(otp_request.username, username,
+                       sizeof(otp_request.username)) >=
+               sizeof(otp_request.username)) {
                AUTH("rlm_otp: username [%s] too long", username);
                return RLM_MODULE_REJECT;
        }
-
-       /* we already know challenge is short enough */
-       otp_request.version = 2;
-
-       strcpy(otp_request.username, username);
-       strcpy(otp_request.challenge, challenge);
+       if (strlcpy(otp_request.challenge, challenge,
+                       sizeof(otp_request.challenge)) >=
+               sizeof(otp_request.challenge)) {
+               AUTH("rlm_otp: challenge for [%s] too long", username);
+               return RLM_MODULE_REJECT;
+       }
 
        otp_request.pwe.pwe = pwe;
 
@@ -112,14 +116,14 @@ int otp_pw_valid(REQUEST *request, int pwe, char const *challenge,
         */
        switch (otp_request.pwe.pwe) {
        case PWE_PAP:
-               if (rvp->length >= sizeof(otp_request.pwe.u.pap.passcode)) {
+               if (strlcpy(otp_request.pwe.u.pap.passcode, rvp->vp_strvalue,
+                               sizeof(otp_request.pwe.u.pap.passcode)) >=
+                       sizeof(otp_request.pwe.u.pap.passcode)) {
                        AUTH("rlm_otp: passcode for [%s] too long",
                               username);
 
                        return RLM_MODULE_REJECT;
                }
-
-               (void) strcpy(otp_request.pwe.u.pap.passcode, rvp->vp_strvalue);
                break;
 
        case PWE_CHAP: