Re: [Opensc-devel] Fwd: New Defects reported by Coverity Scan for OpenSC

[Martin P. <ma...@ma...>]

There are lots of new warnings that could be detected even with clang or
gcc with more warnings.

Sent from a device without a  proper keyboard.
On 5 May 2013 16:58, "Ludovic Rousseau" <lud...@gm...> wrote:

> Hi,
>
> Please find the latest report on new defect(s) introduced to OpenSC
> found with Coverity SCAN
>
> Defect(s) Reported-by: Coverity Scan
> Showing 7 of 43 defects
>
> ** CID 1019105: Destination buffer too small (STRING_OVERFLOW)
>
> http://scan5.coverity.com:8080//sourcebrowser.htm?projectId=10104#mergedDefectId=1019105
>
> ** CID 1019104: Wrong sizeof argument (SIZEOF_MISMATCH)
>
> http://scan5.coverity.com:8080//sourcebrowser.htm?projectId=10104#mergedDefectId=1019104
>
> ** CID 1019103: Resource leak (RESOURCE_LEAK)
>
> http://scan5.coverity.com:8080//sourcebrowser.htm?projectId=10104#mergedDefectId=1019103
>
> ** CID 1019102: Resource leak (RESOURCE_LEAK)
>
> http://scan5.coverity.com:8080//sourcebrowser.htm?projectId=10104#mergedDefectId=1019102
>
> ** CID 1019101: Printf arg count mismatch (PW.TOO_MANY_PRINTF_ARGS)
>
> http://scan5.coverity.com:8080//sourcebrowser.htm?projectId=10104#mergedDefectId=1019101
>
> ** CID 1019100: Printf arg count mismatch (PW.TOO_MANY_PRINTF_ARGS)
>
> http://scan5.coverity.com:8080//sourcebrowser.htm?projectId=10104#mergedDefectId=1019100
>
> ** CID 1019099: Improper use of negative value (NEGATIVE_RETURNS)
>
> http://scan5.coverity.com:8080//sourcebrowser.htm?projectId=10104#mergedDefectId=1019099
>
>
> These bugs are only the NEW bugs since my last submission (a few
> months ago). Many other bugs should also be fixed.
>
> The URL above can't be used if you do not have an account on coverity.
> I would be happy to open an account for you if:
> - you are already a developper of OpenSC
> - you plan to fix bugs detected by coverity
>
> Bye
>
> ________________________________________________________________________
> CID 1019105: Destination buffer too small (STRING_OVERFLOW)
>
> /src/tools/util.c: 316 ( string_overflow)
>    313                                  sprintf(buf + 3, "#%d",
> e->key_ref);
>    314                          break;
>    315                  case SC_AC_SCB:
> >>> You might overrun the 10 byte destination string "buf" by writing 17
> bytes from ""Sec.ControlByte "".
>    316                          strcpy(buf, "Sec.ControlByte ");
>    317                          if (e->key_ref != SC_AC_KEY_REF_NONE)
>    318                                  sprintf(buf + 3, "Ox%X",
> e->key_ref);
>    319                          break;
>    320                  case SC_AC_IDA:
>
>
> /src/tools/util.c: 321 ( string_overflow)
>    318                                  sprintf(buf + 3, "Ox%X",
> e->key_ref);
>    319                          break;
>    320                  case SC_AC_IDA:
> >>> You might overrun the 10 byte destination string "buf" by writing 16
> bytes from ""PKCS#15 AuthID "".
>    321                          strcpy(buf, "PKCS#15 AuthID ");
>    322                          if (e->key_ref != SC_AC_KEY_REF_NONE)
>    323                                  sprintf(buf + 3, "#%d",
> e->key_ref);
>    324                          break;
>    325                  default:
>
> ________________________________________________________________________
> CID 1019104: Wrong sizeof argument (SIZEOF_MISMATCH)
>
> /src/tools/sc-hsm-tool.c: 145 ( suspicious_sizeof)
>    142          int bits = 0;
>    143
>    144          // Seed the RNG
> >>> Passing argument "rngSeed" of type "char *" and argument "8 /* sizeof
> (rngSeed) */" to function "RAND_seed" is suspicious.
>    145          RAND_seed(rngSeed, sizeof(rngSeed));
>    146
>    147          // Determine minimum number of bits for prime >= max(2^r,
> n + 1)
>    148          bits = BN_num_bits_word(n + 1) > BN_num_bits(s) ?
> (BN_num_bits_word(n + 1)) : (BN_num_bits(s));
>    149
>
> ________________________________________________________________________
> CID 1019103: Resource leak (RESOURCE_LEAK)
>
> /src/tools/sc-hsm-tool.c: 288 ( alloc_fn)
>    285          unsigned char j;
>    286
>    287          // Array representing the polynomial a(x) = s + a_1 *
> x + ... + a_n-1 * x^n-1 mod p
> >>> Calling allocation function "malloc".
>    288          BIGNUM **bValue = malloc(t * sizeof(BIGNUM *));
>    289          BIGNUM **pbValue;
>    290          BIGNUM numerator;
>    291          BIGNUM denominator;
>    292          BIGNUM temp;
>
>
> /src/tools/sc-hsm-tool.c: 288 ( var_assign)
>    285          unsigned char j;
>    286
>    287          // Array representing the polynomial a(x) = s + a_1 *
> x + ... + a_n-1 * x^n-1 mod p
> >>> Assigning: "bValue" =  storage returned from "malloc(t * 8UL)".
>    288          BIGNUM **bValue = malloc(t * sizeof(BIGNUM *));
>    289          BIGNUM **pbValue;
>    290          BIGNUM numerator;
>    291          BIGNUM denominator;
>    292          BIGNUM temp;
>
>
> /src/tools/sc-hsm-tool.c: 298 ( var_assign)
>    295          BN_CTX *ctx;
>    296
>    297          // Initialize
> >>> Assigning: "pbValue" = "bValue".
>    298          pbValue = bValue;
>    299          for (i = 0; i < t; i++) {
>    300                  *pbValue = BN_new();
>    301                  BN_init(*pbValue);
>    302                  pbValue++;
>
>
> /src/tools/sc-hsm-tool.c: 313 ( overwrite_var)
>    310          ctx = BN_CTX_new();
>    311          BN_CTX_init(ctx);
>    312
> >>> Overwriting "pbValue" in call "pbValue = bValue" leaks the storage
> that "pbValue" points to.
>    313          pbValue = bValue;
>    314          sp_i = shares;
>    315          for (i = 0; i < t; i++) {
>    316
>    317                  BN_one(&numerator);
>
>
> /src/tools/sc-hsm-tool.c: 313 ( var_assign)
>    310          ctx = BN_CTX_new();
>    311          BN_CTX_init(ctx);
>    312
> >>> Assigning: "pbValue" = "bValue".
>    313          pbValue = bValue;
>    314          sp_i = shares;
>    315          for (i = 0; i < t; i++) {
>    316
>    317                  BN_one(&numerator);
>
>
> /src/tools/sc-hsm-tool.c: 341 ( leaked_storage)
>    338                   * multiplication
>    339                   */
>    340                  if (BN_mod_inverse(&denominator, &denominator,
> &prime, ctx) == NULL ) {
> >>> Variable "bValue" going out of scope leaks the storage it points to.
>    341                          return -1;
>    342                  }
>    343
>    344                  BN_mod_mul(*pbValue, &numerator, &denominator,
> &prime, ctx);
>    345
>
>
> /src/tools/sc-hsm-tool.c: 341 ( leaked_storage)
>    338                   * multiplication
>    339                   */
>    340                  if (BN_mod_inverse(&denominator, &denominator,
> &prime, ctx) == NULL ) {
> >>> Variable "pbValue" going out of scope leaks the storage it points to.
>    341                          return -1;
>    342                  }
>    343
>    344                  BN_mod_mul(*pbValue, &numerator, &denominator,
> &prime, ctx);
>    345
>
> ________________________________________________________________________
> CID 1019102: Resource leak (RESOURCE_LEAK)
>
> /src/libopensc/card.c: 70 ( alloc_fn)
>    67           struct sc_apdu *apdu = NULL;
>    68
>    69           assert(copy_from != NULL);
> >>> Calling allocation function "malloc".
>    70           apdu = (struct sc_apdu *)malloc(sizeof(struct sc_apdu));
>    71           if (!copy_from || !apdu)
>    72                   return apdu;
>    73           memcpy(apdu, copy_from, sizeof(struct sc_apdu));
>    74           apdu->data = apdu->resp = NULL;
>
>
> /src/libopensc/card.c: 70 ( var_assign)
>    67           struct sc_apdu *apdu = NULL;
>    68
>    69           assert(copy_from != NULL);
> >>> Assigning: "apdu" =  storage returned from "malloc(104UL)".
>    70           apdu = (struct sc_apdu *)malloc(sizeof(struct sc_apdu));
>    71           if (!copy_from || !apdu)
>    72                   return apdu;
>    73           memcpy(apdu, copy_from, sizeof(struct sc_apdu));
>    74           apdu->data = apdu->resp = NULL;
>
>
> /src/libopensc/card.c: 73 ( noescape)
>    70           apdu = (struct sc_apdu *)malloc(sizeof(struct sc_apdu));
>    71           if (!copy_from || !apdu)
>    72                   return apdu;
> >>> Variable "apdu" is not freed or pointed-to in function "memcpy".
>    73           memcpy(apdu, copy_from, sizeof(struct sc_apdu));
>    74           apdu->data = apdu->resp = NULL;
>    75           apdu->next = NULL;
>    76           apdu->datalen = apdu->resplen = 0;
>    77           apdu->allocation_flags = SC_APDU_ALLOCATE_FLAG;
>
>
> /src/libopensc/card.c: 82 ( leaked_storage)
>    79           if ((flags & SC_APDU_ALLOCATE_FLAG_DATA) &&
> copy_from->data && copy_from->datalen)   {
>    80                   apdu->data = malloc(copy_from->datalen);
>    81                   if (!apdu->data)
> >>> Variable "apdu" going out of scope leaks the storage it points to.
>    82                           return NULL;
>    83                   memcpy(apdu->data, copy_from->data,
> copy_from->datalen);
>    84                   apdu->datalen = copy_from->datalen;
>    85                   apdu->allocation_flags |=
> SC_APDU_ALLOCATE_FLAG_DATA;
>    86           }
>
>
> /src/libopensc/card.c: 91 ( leaked_storage)
>    88           if ((flags & SC_APDU_ALLOCATE_FLAG_RESP) &&
> copy_from->resp && copy_from->resplen)   {
>    89                   apdu->resp = malloc(copy_from->resplen);
>    90                   if (!apdu->resp)
> >>> Variable "apdu" going out of scope leaks the storage it points to.
>    91                           return NULL;
>    92                   memcpy(apdu->resp, copy_from->resp,
> copy_from->resplen);
>    93                   apdu->resplen = copy_from->resplen;
>    94                   apdu->allocation_flags |=
> SC_APDU_ALLOCATE_FLAG_RESP;
>    95           }
>
> ________________________________________________________________________
> CID 1019101: Printf arg count mismatch (PW.TOO_MANY_PRINTF_ARGS)
>
> /src/tools/sc-hsm-tool.c: 839 ( too_many_printf_args)
>    836           */
>    837          r = sc_get_challenge(card, rngseed, 16);
>    838          if (r < 0) {
> >>> the format string ends before this argument
>    839                  printf("Error generating random seed failed
> with ", sc_strerror(r));
>    840                  OPENSSL_cleanse(pwd, *pwdlen);
>    841                  free(pwd);
>    842                  return r;
>    843          }
>
> ________________________________________________________________________
> CID 1019100: Printf arg count mismatch (PW.TOO_MANY_PRINTF_ARGS)
>
> /src/tools/sc-hsm-tool.c: 816 ( too_many_printf_args)
>    813
>    814          r = sc_get_challenge(card, *pwd, 8);
>    815          if (r < 0) {
> >>> the format string ends before this argument
>    816                  printf("Error generating random key failed
> with ", sc_strerror(r));
>    817                  OPENSSL_cleanse(pwd, *pwdlen);
>    818                  free(pwd);
>    819                  return r;
>    820          }
>
> ________________________________________________________________________
> CID 1019099: Improper use of negative value (NEGATIVE_RETURNS)
>
> /src/tools/sc-hsm-tool.c: 1352 ( var_tested_neg)
>    1349         int opt_dkek_shares = -1;
>    1350         int opt_key_reference = -1;
>    1351         int opt_password_shares_threshold = -1;
> >>> Assigning: "opt_password_shares_total" = a negative value.
>    1352         int opt_password_shares_total = -1;
>    1353         int opt_force = 0;
>    1354         int opt_iter = 10000000;
>    1355         sc_context_param_t ctx_param;
>    1356
>
>
> /src/tools/sc-hsm-tool.c: 1468 ( negative_returns)
>    1465         }
>    1466
>    1467         if (do_create_dkek_share) {
> >>> "opt_password_shares_total" is passed to a parameter that cannot be
> negative.
>    1468                 create_dkek_share(card, opt_filename,
> opt_iter, opt_password, opt_password_shares_threshold,
> opt_password_shares_total);
>    1469         }
>    1470
>    1471         if (do_import_dkek_share) {
>    1472                 import_dkek_share(card, opt_filename,
> opt_iter, opt_password, opt_password_shares_total);
>
> ________________________________________________________________________
>
>
>
> --
>  Dr. Ludovic Rousseau
>
>
> ------------------------------------------------------------------------------
> Get 100% visibility into Java/.NET code with AppDynamics Lite
> It's a free troubleshooting tool designed for production
> Get down to code-level detail for bottlenecks, with <2% overhead.
> Download for free and get started troubleshooting in minutes.
> http://p.sf.net/sfu/appdyn_d2d_ap2
> _______________________________________________
> Opensc-devel mailing list
> Ope...@li...
> https://lists.sourceforge.net/lists/listinfo/opensc-devel
>