Re: [openssl-project] coverity defect release criteria (Fwd: New Defects reported by Coverity Scan for openssl/openssl)

2018-09-09 Thread Dr. Matthias St. Pierre


> > *** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > work in progress...
> 
> I think this one may be a false positive -- it's worried that EVP_MD_size()
> will return -1, but we've essentially already validated that the md is
> valid by the time we get there.  I didn't do a full check, though.
> 
> -Ben

Yes, that's my suspicion, too. But I am also not sure yet.
As far as I understand it, EVP_MD_size() will be negative only if md == NULL. 
So it boils down to the question whether one can assert that mctx
always contains a valid md in line 261:

const EVP_MD *md = EVP_MD_CTX_md(mctx);

If that is the case, then one can silence coverity by casting the sign of the
return value of EVP_MD_size(). But if not, some error handling is missing.

Matthias


___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


Re: [openssl-project] coverity defect release criteria (Fwd: New Defects reported by Coverity Scan for openssl/openssl)

2018-09-09 Thread Benjamin Kaduk
On Sun, Sep 09, 2018 at 10:38:50PM +, Dr. Matthias St. Pierre wrote:
> preliminary status report:
> 
> *** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
>   see https://github.com/openssl/openssl/pull/7156
>   
> *** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
>   work in progress...   

I think this one may be a false positive -- it's worried that EVP_MD_size()
will return -1, but we've essentially already validated that the md is
valid by the time we get there.  I didn't do a full check, though.

-Ben

> *** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
>   see https://github.com/openssl/openssl/pull/7155
> 
> *** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
>   todo
> 
> *** CID 1423323:  Null pointer dereferences  (FORWARD_NULL)
>   see https://github.com/openssl/openssl/pull/7158
> 
> *** CID 1201571:  Error handling issues  (CHECKED_RETURN)
>   todo
> 
> if anybody wants to fix one of the CIDs marked 'todo', no problem. Just drop 
> a note on the openssl-project list.
> 
> Matthias
> 
> 
> > -Ursprüngliche Nachricht-
> > Von: openssl-project  Im Auftrag von 
> > Benjamin Kaduk
> > Gesendet: Sonntag, 9. September 2018 18:04
> > An: openssl-project@openssl.org
> > Betreff: [openssl-project] coverity defect release criteria (Fwd: New 
> > Defects reported by Coverity Scan for openssl/openssl)
> > 
> > I see that Matthias has opened pull requests for a couple of these already;
> > are you planning to work through the rest of them as well?
> > 
> > -Ben
> > 
> > On Sun, Sep 09, 2018 at 09:28:12AM +, scan-ad...@coverity.com wrote:
> > > Hi,
> > >
> > > Please find the latest report on new defect(s) introduced to 
> > > openssl/openssl found with Coverity Scan.
> > >
> > > 6 new defect(s) introduced to openssl/openssl found with Coverity Scan.
> > >
> > >
> > > New defect(s) Reported-by: Coverity Scan
> > > Showing 6 of 6 defect(s)
> > >
> > >
> > > ** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> > >
> > >
> > > 
> > > *** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> > > /crypto/rsa/rsa_pss.c: 247 in RSA_padding_add_PKCS1_PSS_mgf1()
> > > 241 EM[emLen - 1] = 0xbc;
> > > 242
> > > 243 ret = 1;
> > > 244
> > > 245  err:
> > > 246 EVP_MD_CTX_free(ctx);
> > > >>> CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> > > >>> "sLen" is passed to a parameter that cannot be negative.
> > > 247 OPENSSL_clear_free(salt, sLen);
> > > 248
> > > 249 return ret;
> > > 250
> > > 251 }
> > > 252
> > > 253 #if defined(_MSC_VER)
> > > 254 # pragma optimize("",on)
> > >
> > > ** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > >
> > >
> > > 
> > > *** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > > /crypto/sm2/sm2_pmeth.c: 277 in pkey_sm2_digest_custom()
> > > 271 }
> > > 272
> > > 273 /* get hashed prefix 'z' of tbs message */
> > > 274 if (!sm2_compute_z_digest(z, md, smctx->id, smctx->id_len, 
> > > ec))
> > > 275 return 0;
> > > 276
> > > >>> CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > > >>> "EVP_MD_size(md)" is passed to a parameter that cannot be 
> > > >>> negative.
> > > 277 return EVP_DigestUpdate(mctx, z, EVP_MD_size(md));
> > > 278 }
> > > 279
> > > 280 const EVP_PKEY_METHOD sm2_pkey_meth = {
> > > 281 EVP_PKEY_SM2,
> > > 282 0,
> > >
> > > ** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > > /test/dhtest.c: 202 in dh_test()
> > >
> > >
> > > 
> > > *** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > > /test/dhtest.c: 202 in dh_test()
> > > 196 BN_free(bp);
> > > 197 BN_free(bg);
> > > 198 BN_free(cpriv_key);
> > > 199 BN_GENCB_free(_cb);
> > > 200 DH_free(dh);
> > > 201
> > > >>> CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > > >>> Variable "priv_key" going out of scope leaks the storage it 
> > > >>> points to.
> > > 202 return ret;
> > > 203 }
> > > 204
> > > 205 static int cb(int p, int n, BN_GENCB *arg)
> > > 206 {
> > > 207 return 1;
> > >
> > > ** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> > >
> > >
> > > 
> > > *** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> > > /apps/speed.c: 3105 in speed_main()
> > > 3099 ERR_print_errors(bio_err);
> > > 3100 rsa_count = 1;
> > > 3101 } else {
> > > 3102 for (i = 0; i 

Re: [openssl-project] coverity defect release criteria (Fwd: New Defects reported by Coverity Scan for openssl/openssl)

2018-09-09 Thread Dr. Matthias St. Pierre
preliminary status report:

*** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
see https://github.com/openssl/openssl/pull/7156

*** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
work in progress...   

*** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
see https://github.com/openssl/openssl/pull/7155

*** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
todo

*** CID 1423323:  Null pointer dereferences  (FORWARD_NULL)
see https://github.com/openssl/openssl/pull/7158

*** CID 1201571:  Error handling issues  (CHECKED_RETURN)
todo

if anybody wants to fix one of the CIDs marked 'todo', no problem. Just drop a 
note on the openssl-project list.

Matthias


> -Ursprüngliche Nachricht-
> Von: openssl-project  Im Auftrag von 
> Benjamin Kaduk
> Gesendet: Sonntag, 9. September 2018 18:04
> An: openssl-project@openssl.org
> Betreff: [openssl-project] coverity defect release criteria (Fwd: New Defects 
> reported by Coverity Scan for openssl/openssl)
> 
> I see that Matthias has opened pull requests for a couple of these already;
> are you planning to work through the rest of them as well?
> 
> -Ben
> 
> On Sun, Sep 09, 2018 at 09:28:12AM +, scan-ad...@coverity.com wrote:
> > Hi,
> >
> > Please find the latest report on new defect(s) introduced to 
> > openssl/openssl found with Coverity Scan.
> >
> > 6 new defect(s) introduced to openssl/openssl found with Coverity Scan.
> >
> >
> > New defect(s) Reported-by: Coverity Scan
> > Showing 6 of 6 defect(s)
> >
> >
> > ** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> >
> >
> > 
> > *** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> > /crypto/rsa/rsa_pss.c: 247 in RSA_padding_add_PKCS1_PSS_mgf1()
> > 241 EM[emLen - 1] = 0xbc;
> > 242
> > 243 ret = 1;
> > 244
> > 245  err:
> > 246 EVP_MD_CTX_free(ctx);
> > >>> CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> > >>> "sLen" is passed to a parameter that cannot be negative.
> > 247 OPENSSL_clear_free(salt, sLen);
> > 248
> > 249 return ret;
> > 250
> > 251 }
> > 252
> > 253 #if defined(_MSC_VER)
> > 254 # pragma optimize("",on)
> >
> > ** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> >
> >
> > 
> > *** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > /crypto/sm2/sm2_pmeth.c: 277 in pkey_sm2_digest_custom()
> > 271 }
> > 272
> > 273 /* get hashed prefix 'z' of tbs message */
> > 274 if (!sm2_compute_z_digest(z, md, smctx->id, smctx->id_len, ec))
> > 275 return 0;
> > 276
> > >>> CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > >>> "EVP_MD_size(md)" is passed to a parameter that cannot be negative.
> > 277 return EVP_DigestUpdate(mctx, z, EVP_MD_size(md));
> > 278 }
> > 279
> > 280 const EVP_PKEY_METHOD sm2_pkey_meth = {
> > 281 EVP_PKEY_SM2,
> > 282 0,
> >
> > ** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > /test/dhtest.c: 202 in dh_test()
> >
> >
> > 
> > *** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > /test/dhtest.c: 202 in dh_test()
> > 196 BN_free(bp);
> > 197 BN_free(bg);
> > 198 BN_free(cpriv_key);
> > 199 BN_GENCB_free(_cb);
> > 200 DH_free(dh);
> > 201
> > >>> CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > >>> Variable "priv_key" going out of scope leaks the storage it points 
> > >>> to.
> > 202 return ret;
> > 203 }
> > 204
> > 205 static int cb(int p, int n, BN_GENCB *arg)
> > 206 {
> > 207 return 1;
> >
> > ** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> >
> >
> > 
> > *** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> > /apps/speed.c: 3105 in speed_main()
> > 3099 ERR_print_errors(bio_err);
> > 3100 rsa_count = 1;
> > 3101 } else {
> > 3102 for (i = 0; i < loopargs_len; i++) {
> > 3103 /* Perform EdDSA signature test */
> > 3104 loopargs[i].siglen = 
> > test_ed_curves[testnum].siglen;
> > >>> CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> > >>> Pointer "[i].siglen" points to an object whose effective 
> > >>> type is "unsigned int" (32 bits, unsigned) but is dereferenced as a
> wider "unsigned long" (64 bits, unsigned).  This may lead to memory 
> corruption.
> > 3105 st = 

Re: [openssl-project] coverity defect release criteria (Fwd: New Defects reported by Coverity Scan for openssl/openssl)

2018-09-09 Thread Dr. Matthias St. Pierre
I am currently occupied with other things, so I won't be able to look at it 
before later this evening or tomorrow.

I also had a quick look at CID 1423323 (see below) but I was unable to see why 
'pkey' would be a NULL pointer
when passed to 'EVP_PKEY_up_ref'. So I'm not sure yet whether this is a false 
positive.

Matthias


> -Ursprüngliche Nachricht-
> Von: openssl-project  Im Auftrag von 
> Benjamin Kaduk
> Gesendet: Sonntag, 9. September 2018 18:04
> An: openssl-project@openssl.org
> Betreff: [openssl-project] coverity defect release criteria (Fwd: New Defects 
> reported by Coverity Scan for openssl/openssl)
> 
> I see that Matthias has opened pull requests for a couple of these already;
> are you planning to work through the rest of them as well?
> 
> -Ben
> 
> On Sun, Sep 09, 2018 at 09:28:12AM +, scan-ad...@coverity.com wrote:
> > Hi,
> >
> > Please find the latest report on new defect(s) introduced to 
> > openssl/openssl found with Coverity Scan.
> >
> > 6 new defect(s) introduced to openssl/openssl found with Coverity Scan.
> >
> >
> > New defect(s) Reported-by: Coverity Scan
> > Showing 6 of 6 defect(s)
> >
> >
> > ** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> >
> >
> > 
> > *** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> > /crypto/rsa/rsa_pss.c: 247 in RSA_padding_add_PKCS1_PSS_mgf1()
> > 241 EM[emLen - 1] = 0xbc;
> > 242
> > 243 ret = 1;
> > 244
> > 245  err:
> > 246 EVP_MD_CTX_free(ctx);
> > >>> CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> > >>> "sLen" is passed to a parameter that cannot be negative.
> > 247 OPENSSL_clear_free(salt, sLen);
> > 248
> > 249 return ret;
> > 250
> > 251 }
> > 252
> > 253 #if defined(_MSC_VER)
> > 254 # pragma optimize("",on)
> >
> > ** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> >
> >
> > 
> > *** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > /crypto/sm2/sm2_pmeth.c: 277 in pkey_sm2_digest_custom()
> > 271 }
> > 272
> > 273 /* get hashed prefix 'z' of tbs message */
> > 274 if (!sm2_compute_z_digest(z, md, smctx->id, smctx->id_len, ec))
> > 275 return 0;
> > 276
> > >>> CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> > >>> "EVP_MD_size(md)" is passed to a parameter that cannot be negative.
> > 277 return EVP_DigestUpdate(mctx, z, EVP_MD_size(md));
> > 278 }
> > 279
> > 280 const EVP_PKEY_METHOD sm2_pkey_meth = {
> > 281 EVP_PKEY_SM2,
> > 282 0,
> >
> > ** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > /test/dhtest.c: 202 in dh_test()
> >
> >
> > 
> > *** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > /test/dhtest.c: 202 in dh_test()
> > 196 BN_free(bp);
> > 197 BN_free(bg);
> > 198 BN_free(cpriv_key);
> > 199 BN_GENCB_free(_cb);
> > 200 DH_free(dh);
> > 201
> > >>> CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> > >>> Variable "priv_key" going out of scope leaks the storage it points 
> > >>> to.
> > 202 return ret;
> > 203 }
> > 204
> > 205 static int cb(int p, int n, BN_GENCB *arg)
> > 206 {
> > 207 return 1;
> >
> > ** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> >
> >
> > 
> > *** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> > /apps/speed.c: 3105 in speed_main()
> > 3099 ERR_print_errors(bio_err);
> > 3100 rsa_count = 1;
> > 3101 } else {
> > 3102 for (i = 0; i < loopargs_len; i++) {
> > 3103 /* Perform EdDSA signature test */
> > 3104 loopargs[i].siglen = 
> > test_ed_curves[testnum].siglen;
> > >>> CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> > >>> Pointer "[i].siglen" points to an object whose effective 
> > >>> type is "unsigned int" (32 bits, unsigned) but is dereferenced as a
> wider "unsigned long" (64 bits, unsigned).  This may lead to memory 
> corruption.
> > 3105 st = EVP_DigestSign(loopargs[i].eddsa_ctx[testnum],
> > 3106 loopargs[i].buf2, (size_t 
> > *)[i].siglen,
> > 3107 loopargs[i].buf, 20);
> > 3108 if (st == 0)
> > 3109 break;
> > 3110 }
> >
> > ** CID 1423323:  Null pointer dereferences  (FORWARD_NULL)
> >
> >
> > 

[openssl-project] coverity defect release criteria (Fwd: New Defects reported by Coverity Scan for openssl/openssl)

2018-09-09 Thread Benjamin Kaduk
I see that Matthias has opened pull requests for a couple of these already;
are you planning to work through the rest of them as well?

-Ben

On Sun, Sep 09, 2018 at 09:28:12AM +, scan-ad...@coverity.com wrote:
> Hi,
> 
> Please find the latest report on new defect(s) introduced to openssl/openssl 
> found with Coverity Scan.
> 
> 6 new defect(s) introduced to openssl/openssl found with Coverity Scan.
> 
> 
> New defect(s) Reported-by: Coverity Scan
> Showing 6 of 6 defect(s)
> 
> 
> ** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> 
> 
> 
> *** CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> /crypto/rsa/rsa_pss.c: 247 in RSA_padding_add_PKCS1_PSS_mgf1()
> 241 EM[emLen - 1] = 0xbc;
> 242 
> 243 ret = 1;
> 244 
> 245  err:
> 246 EVP_MD_CTX_free(ctx);
> >>> CID 1439138:  Integer handling issues  (NEGATIVE_RETURNS)
> >>> "sLen" is passed to a parameter that cannot be negative.
> 247 OPENSSL_clear_free(salt, sLen);
> 248 
> 249 return ret;
> 250 
> 251 }
> 252 
> 253 #if defined(_MSC_VER)
> 254 # pragma optimize("",on)
> 
> ** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> 
> 
> 
> *** CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> /crypto/sm2/sm2_pmeth.c: 277 in pkey_sm2_digest_custom()
> 271 }
> 272 
> 273 /* get hashed prefix 'z' of tbs message */
> 274 if (!sm2_compute_z_digest(z, md, smctx->id, smctx->id_len, ec))
> 275 return 0;
> 276 
> >>> CID 1439137:  Integer handling issues  (NEGATIVE_RETURNS)
> >>> "EVP_MD_size(md)" is passed to a parameter that cannot be negative.
> 277 return EVP_DigestUpdate(mctx, z, EVP_MD_size(md));
> 278 }
> 279 
> 280 const EVP_PKEY_METHOD sm2_pkey_meth = {
> 281 EVP_PKEY_SM2,
> 282 0,
> 
> ** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> /test/dhtest.c: 202 in dh_test()
> 
> 
> 
> *** CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> /test/dhtest.c: 202 in dh_test()
> 196 BN_free(bp);
> 197 BN_free(bg);
> 198 BN_free(cpriv_key);
> 199 BN_GENCB_free(_cb);
> 200 DH_free(dh);
> 201 
> >>> CID 1439136:  Resource leaks  (RESOURCE_LEAK)
> >>> Variable "priv_key" going out of scope leaks the storage it points to.
> 202 return ret;
> 203 }
> 204 
> 205 static int cb(int p, int n, BN_GENCB *arg)
> 206 {
> 207 return 1;
> 
> ** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> 
> 
> 
> *** CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> /apps/speed.c: 3105 in speed_main()
> 3099 ERR_print_errors(bio_err);
> 3100 rsa_count = 1;
> 3101 } else {
> 3102 for (i = 0; i < loopargs_len; i++) {
> 3103 /* Perform EdDSA signature test */
> 3104 loopargs[i].siglen = test_ed_curves[testnum].siglen;
> >>> CID 1439135:  Memory - illegal accesses  (INCOMPATIBLE_CAST)
> >>> Pointer "[i].siglen" points to an object whose effective 
> >>> type is "unsigned int" (32 bits, unsigned) but is dereferenced as a wider 
> >>> "unsigned long" (64 bits, unsigned).  This may lead to memory corruption.
> 3105 st = EVP_DigestSign(loopargs[i].eddsa_ctx[testnum],
> 3106 loopargs[i].buf2, (size_t 
> *)[i].siglen,
> 3107 loopargs[i].buf, 20);
> 3108 if (st == 0)
> 3109 break;
> 3110 }
> 
> ** CID 1423323:  Null pointer dereferences  (FORWARD_NULL)
> 
> 
> 
> *** CID 1423323:  Null pointer dereferences  (FORWARD_NULL)
> /test/evp_extra_test.c: 894 in test_EVP_PKEY_check()
> 888 
> 889 if (!TEST_int_eq(EVP_PKEY_param_check(ctx), expected_param_check))
> 890 goto done;
> 891 
> 892 ctx2 = EVP_PKEY_CTX_new_id(0xdefaced, NULL);
> 893 /* assign the pkey directly, as an internal test */
> >>> CID 1423323:  Null pointer dereferences  (FORWARD_NULL)
> >>> Passing null pointer "pkey" to "EVP_PKEY_up_ref", which dereferences 
> >>> it.
> 894 EVP_PKEY_up_ref(pkey);
> 895 ctx2->pkey = pkey;
> 896 
> 897 if (!TEST_int_eq(EVP_PKEY_check(ctx2), 0xbeef))
> 898 goto done;
> 899 
> 
> ** CID 1201571:  Error handling issues  

Re: [openssl-project] Please freeze the repo

2018-09-09 Thread Richard Levitte
In message <22962ad7-6232-dcd7-4ec4-11544360f...@openssl.org> on Sun, 9 Sep 
2018 11:34:18 +0100, Matt Caswell  said:

> Please can someone freeze the repo:
> 
> ssh openssl-...@git.openssl.org freeze openssl matt

Done

-- 
Richard Levitte levi...@openssl.org
OpenSSL Project http://www.openssl.org/~levitte/
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project


[openssl-project] Please freeze the repo

2018-09-09 Thread Matt Caswell
Please can someone freeze the repo:

ssh openssl-...@git.openssl.org freeze openssl matt


Thanks

Matt
___
openssl-project mailing list
openssl-project@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-project