crypto: fix return type prob reported by coverity

[mhdawson]

Coverity correctly reported that the value returned
by BIO_get_mem_data could be negative and the type
provided for the return value was unsigned.

Fix up the type and check.

Signed-off-by: Michael Dawson mdawson@devrus.com

[nodejs-github-bot]

Review requested:

• @nodejs/crypto

[mhdawson]

This is what was reported by Coverity

1 new defect(s) introduced to Node.js found with Coverity Scan.


New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)


** CID 249964:  Integer handling issues  (NEGATIVE_RETURNS)


________________________________________________________________________________________________________
*** CID 249964:  Integer handling issues  (NEGATIVE_RETURNS)
/src/crypto/crypto_common.cc: 772 in node::crypto::PrintGeneralName(const std::unique_ptr<bio_st, node::FunctionDeleter<bio_st, (&BIO_free_all)>> &, const GENERAL_NAME_st *)()
766                                kX509NameFlagsRFC2253WithinUtf8JSON) < 0) {
767           return false;
768         }
769         char* oline = nullptr;
770         size_t n_bytes = BIO_get_mem_data(tmp.get(), &oline);
771         CHECK_IMPLIES(n_bytes != 0, oline != nullptr);
>>>     CID 249964:  Integer handling issues  (NEGATIVE_RETURNS)
>>>     "n_bytes" is passed to a parameter that cannot be negative.
772         PrintAltName(out, oline, n_bytes, true, nullptr);
773       } else if (gen->type == GEN_IPADD) {
774         BIO_printf(out.get(), "IP Address:");
775         const ASN1_OCTET_STRING* ip = gen->d.ip;
776         const unsigned char* b = ip->data;
777         if (ip->length == 4) {

[mhdawson]

I don't know whey I don't get the linter errors when I run locally :(

[mhdawson]

Fixed lint issues and force pushed.

[tniessen]

FWIW, BIO_get_mem_data internally casts a size_t to long, so the result is guaranteed to be valid when cast to size_t.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/42818/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/42819/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/42821/

[mhdawson]

The update to the checks on the issue seems to be stale. This CI run - https://ci.nodejs.org/job/node-test-pull-request/42821/ shows all green. Going to land.

[mhdawson]

Landed in a9c0689

[bengl]

This doesn't land on 17.x, and it looks like it's because it's modifying code added in a semver-major, so I'm adding the dont-land-on-17 label.

[juanarbol]

This depends on #42002 which is another semver major; so this can not be landed in V16.x