Why does this code give redundantCopy?

[Sander Bouwhuis]

I have the following code:

  // Allocate a buffer to hold the data
  i32ChallengeLen = int32(strlen(pi8Challenge));
  pu8Data = new uint8[64 + Max(i32ChallengeLen, int32(sizeof(pu8Digest)))];

  // Calculate key XOR 0x36
  memset(pu8Data, NULL, 64 + i32ChallengeLen);  // redundantCopy warning
  memmove(pu8Data, pu8Key, i32KeyLen);
  for(uint8 u8Pos=0;u8Pos<64;u8Pos++)
    pu8Data[u8Pos] ^= 0x36;

I get a redundantCopy warning on the line with the memset.
Why is that?

[Alexander Mai]

I've wrapped your example into an executable scope (to avoid problems with otherwise invalid code snippet)

void foo() {
    i32ChallengeLen = int32(strlen(pi8Challenge));
    pu8Data = new uint8[64 + Max(i32ChallengeLen, int32(sizeof(pu8Digest)))];
    memset(pu8Data, NULL, 64 + i32ChallengeLen); // redundantCopy warning
    memmove(pu8Data, pu8Key, i32KeyLen);
    for(uint8 u8Pos=0; u8Pos<64; u8Pos++)
        pu8Data[u8Pos] ^= 0x36;
}

and ran cppcheck.1.69 a.cpp --enable=all but don't see your error message.
What version do you use?

[Sander Bouwhuis]

I use v1.69.

I in fact get the warning twice. I've marked it in the source code below.
For completeness sake, here is the complete routine. It's used for authenticating SMTP connections.

// This function tries to authenticate a user
bool CSmtp::Authenticate(const int8 *pi8AuthMethods, const int8 *pi8Username, const int8 *pi8Password) const
{
  bool bResult = false;

  try
  {
    CHECK_MEM_LEAKS

    // ----------------
    //    DIGEST-MD5
    // ----------------

    // Is the DIGEST-MD5 (RFC 2831) authentication method supported?
    if(!bResult && (FindInString(pi8AuthMethods, "DIGEST-MD5") >= 0))
    {
      // Not yet implemented
    }

    // --------------
    //    CRAM-MD5
    // --------------

    // Is the CRAM-MD5 (RFC 2195) authentication method supported?
    if(!bResult && (FindInString(pi8AuthMethods, "CRAM-MD5") >= 0))
    {
      int8 pi8Result[1024];

      // Tell the SMTP server we want to authenticate ourselves using the CRAM-MD5 method
      if(SendCommand("AUTH CRAM-MD5\r\n", "334", pi8Result, _countof(pi8Result)))
      {
        CBase64 base64;
        int8    pi8Challenge[1024];

        // Decode the response ("334 " followed by a Base64 string)
        if(base64.Decode((uint8 *)pi8Challenge, _countof(pi8Challenge), &pi8Result[4]))
        {
          CMd5  md5;
          int32 i32KeyLen, i32ChallengeLen;
          uint8 pu8Key[64], *pu8Data, pu8Digest[16];
          int8  pi8Command[1024], pi8Md5[33], pi8Encoded[512], pi8Uncoded[512];

          // Is the password too long to be used as key?
          i32KeyLen = int32(strlen(pi8Password));
          if(i32KeyLen > 64)
          {
            // Use the MD5 of the password as key
            md5.GetFromBuffer(pi8Password, i32KeyLen, pu8Key);
            i32KeyLen = 16;
          }
          else
            Safe_strncpy((int8 *)pu8Key, sizeof(pu8Key), pi8Password);

          // The HMAC-MD5 transform = MD5(key XOR 0x5c, MD5(key XOR 0x36, challenge))

          // Allocate a buffer to hold the data
          i32ChallengeLen = int32(strlen(pi8Challenge));
          pu8Data = new uint8[64 + Max(i32ChallengeLen, int32(sizeof(pu8Digest)))];

          // Calculate key XOR 0x36
// ------------------------------------------------------
// !!! THE FOLLOWING LINE IS MARKED AS REDUNDANT COPY !!!
// ------------------------------------------------------
          memset(pu8Data, NULL, 64 + i32ChallengeLen);
          memmove(pu8Data, pu8Key, i32KeyLen);
          for(uint8 u8Pos=0;u8Pos<64;u8Pos++)
            pu8Data[u8Pos] ^= 0x36;

          // Append the challenge
          memmove(&pu8Data[64], pi8Challenge, i32ChallengeLen);

          // Calculate MD5(key XOR 0x36, challenge)
          md5.GetFromBuffer(pu8Data, 64 + i32ChallengeLen, pu8Digest);

          // Calculate key XOR 0x5c
// ------------------------------------------------------
// !!! THE FOLLOWING LINE IS MARKED AS REDUNDANT COPY !!!
// ------------------------------------------------------
          memset(pu8Data, NULL, 64 + sizeof(pu8Digest));
          memmove(pu8Data, pu8Key, i32KeyLen);
          for(uint8 u8Pos=0;u8Pos<64;u8Pos++)
            pu8Data[u8Pos] ^= 0x5c;

          // Append the digest
          memmove(&pu8Data[64], pu8Digest, sizeof(pu8Digest));

          // Calculate MD5(key XOR 0x5c, MD5(key XOR 0x36, challenge))
          md5.GetFromBufferAsString(pu8Data, 64 + sizeof(pu8Digest), pi8Md5);

          // Free the resources
          SAFE_DELETE_ARRAY(pu8Data);

          // Append the HMAC digest to the username
          Safe_snprintf(pi8Uncoded, _countof(pi8Uncoded), "%s %s", pi8Username, pi8Md5);

          // Base64 encode the response
          base64.Encode(pi8Encoded, _countof(pi8Encoded), (uint8 *)pi8Uncoded, int32(strlen(pi8Uncoded)), 0);
          Safe_snprintf(pi8Command, _countof(pi8Command), "%s\r\n", pi8Encoded);

          // Send the response to the challenge
          bResult = SendCommand(pi8Command, "235", pi8Result, _countof(pi8Result));
        }
      }
    }

    // -----------
    //    PLAIN
    // -----------

    // Is the PLAIN (RFC 2595) authentication method supported?
    if(!bResult && (FindInString(pi8AuthMethods, "PLAIN") >= 0))
    {
      CBase64 base64;
      int8    pi8Result[1024], pi8Command[1024], pi8Encoded[1024], pi8Uncoded[1024];
      int32   i32Len;

      // Base64 encode the authorization id (not used), the authentication id, and the password
      i32Len = Safe_snprintf(pi8Uncoded, _countof(pi8Uncoded), "%s%c%s%c%s", "", '\0', pi8Username, '\0', pi8Password);
      base64.Encode(pi8Encoded, _countof(pi8Encoded), (uint8 *)pi8Uncoded, i32Len, 0);
      Safe_snprintf(pi8Command, _countof(pi8Command), "AUTH PLAIN %s\r\n", pi8Encoded);

      // Tell the SMTP server we want to authenticate ourselves using the LOGIN method
      bResult = SendCommand(pi8Command, "235", pi8Result, _countof(pi8Result));
    }

    // -----------
    //    LOGIN
    // -----------

    // Is the LOGIN (non-standard) authentication method supported?
    if(!bResult && (FindInString(pi8AuthMethods, "LOGIN") >= 0))
    {
      int8 pi8Result[1024];

      // Tell the SMTP server we want to authenticate ourselves using the LOGIN method
      if(SendCommand("AUTH LOGIN\r\n", "334 VXNlcm5hbWU6", pi8Result, _countof(pi8Result)))
      {
        CBase64 base64;
        int8    pi8Command[1024], pi8Encoded[1024];

        // Base64 encode the username
        base64.Encode(pi8Encoded, _countof(pi8Encoded), (uint8 *)pi8Username, (uint32)strlen(pi8Username), 0);
        Safe_snprintf(pi8Command, _countof(pi8Command), "%s\r\n", pi8Encoded);

        // Send the username
        if(SendCommand(pi8Command, "334 UGFzc3dvcmQ6", pi8Result, _countof(pi8Result)))
        {
          // Base64 encode the password
          base64.Encode(pi8Encoded, _countof(pi8Encoded), (uint8 *)pi8Password, (uint32)strlen(pi8Password), 0);
          Safe_snprintf(pi8Command, _countof(pi8Command), "%s\r\n", pi8Encoded);

          // Send the password
          bResult = SendCommand(pi8Command, "235", pi8Result, _countof(pi8Result));
        }
      }
    }
  }
  catch(...) { OnException(__FILE__, __LINE__, __FUNCSIG__, __TIMESTAMP__); }

  return bResult;
}

[Sander Bouwhuis]

Ok, I fixed it like this:

// Calculate key XOR 0x36
memmove(pu8Data, pu8Key, i32KeyLen);
memset(&pu8Data[i32KeyLen], NULL, 64 + i32ChallengeLen - i32KeyLen);
for(uint8 u8Pos=0;u8Pos<64;u8Pos++)
  pu8Data[u8Pos] ^= 0x36;

[Colin D Bennett]

Also I would suggest not to use NULL as the second argument to memset().

// Calculate key XOR 0x36
memmove(pu8Data, pu8Key, i32KeyLen);
memset(&pu8Data[i32KeyLen], NULL, 64 + i32ChallengeLen - i32KeyLen);
for(uint8 u8Pos=0;u8Pos<64;u8Pos++)
  pu8Data[u8Pos] ^= 0x36;

That parameter is a scalar value (the byte value with which to fill the array), but NULL should be used only in a pointer context. Now often C compilers define NULL as simply 0 but it is a misleading use of NULL -- using nullptr in C++11 would find this and produce a compile error instead of accepting it.