windows: fix issues detected by clang-tidy, and some more #16777
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
```
/home/runner/work/curl/curl/lib/vauth/digest_sspi.c:586:14: error: Potential leak of memory pointed to by 'spn' [clang-analyzer-unix.Malloc,-warnings-as-errors]
586 | return CURLE_OUT_OF_MEMORY;
| ^
```
https://github.com/curl/curl/actions/runs/13968764259/job/39105517451?pr=16764#step:8:159
not detected by clang-tidy. Checkme.
https://github.com/curl/curl/actions/runs/13969308325/job/39106970359?pr=16764#step:8:175 ``` /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:413:18: error: Access to field 'cAltEntry' results in a dereference of a null pointer (loaded from variable 'alt_name_info') [clang-analyzer-core.NullDereference,-warnings-as-errors] 413 | for(i = 0; i < alt_name_info->cAltEntry; i++) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:762:3: note: Loop condition is false. Exiting loop 762 | DEBUGASSERT(BACKEND); | ^ /home/runner/work/curl/curl/lib/curl_setup_once.h:301:24: note: expanded from macro 'DEBUGASSERT' 301 | #define DEBUGASSERT(x) do { } while(0) | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:769:7: note: Assuming 'sspi_status' is equal to SEC_E_OK 769 | if((sspi_status != SEC_E_OK) || !pCertContextServer) { | ^~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:769:6: note: Left side of '||' is false 769 | if((sspi_status != SEC_E_OK) || !pCertContextServer) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:769:35: note: Assuming 'pCertContextServer' is non-null 769 | if((sspi_status != SEC_E_OK) || !pCertContextServer) { | ^~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:769:3: note: Taking false branch 769 | if((sspi_status != SEC_E_OK) || !pCertContextServer) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:777:6: note: 'result' is equal to CURLE_OK 777 | if(result == CURLE_OK && | ^~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:777:6: note: Left side of '&&' is true /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:778:8: note: Assuming field 'CAfile' is null 778 | (conn_config->CAfile || conn_config->ca_info_blob) && | ^~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:778:8: note: Left side of '||' is false /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:778:31: note: Assuming field 'ca_info_blob' is null 778 | (conn_config->CAfile || conn_config->ca_info_blob) && | ^~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:778:58: note: Left side of '&&' is false 778 | (conn_config->CAfile || conn_config->ca_info_blob) && | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:863:6: note: 'result' is equal to CURLE_OK 863 | if(result == CURLE_OK) { | ^~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:863:3: note: Taking true branch 863 | if(result == CURLE_OK) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:874:34: note: Assuming field 'no_revoke' is 0 874 | (ssl_config->no_revoke ? 0 : | ^~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:874:34: note: '?' condition is false /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:869:8: note: Assuming the condition is false 869 | if(!CertGetCertificateChain(cert_chain_engine, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 870 | pCertContextServer, | ~~~~~~~~~~~~~~~~~~~ 871 | NULL, | ~~~~~ 872 | pCertContextServer->hCertStore, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 873 | &ChainPara, | ~~~~~~~~~~~ 874 | (ssl_config->no_revoke ? 0 : | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 875 | CERT_CHAIN_REVOCATION_CHECK_CHAIN), | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 876 | NULL, | ~~~~~ 877 | &pChainContext)) { | ~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:869:5: note: Taking false branch 869 | if(!CertGetCertificateChain(cert_chain_engine, | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:885:8: note: 'result' is equal to CURLE_OK 885 | if(result == CURLE_OK) { | ^~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:885:5: note: Taking true branch 885 | if(result == CURLE_OK) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:890:10: note: Assuming field 'revoke_best_effort' is 0 890 | if(data->set.ssl.revoke_best_effort) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:890:7: note: Taking false branch 890 | if(data->set.ssl.revoke_best_effort) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:898:10: note: Assuming 'dwTrustErrorMask' is 0 898 | if(dwTrustErrorMask) { | ^~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:898:7: note: Taking false branch 898 | if(dwTrustErrorMask) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:922:6: note: 'result' is equal to CURLE_OK 922 | if(result == CURLE_OK) { | ^~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:922:3: note: Taking true branch 922 | if(result == CURLE_OK) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:923:8: note: Assuming field 'verifyhost' is not equal to 0 923 | if(conn_config->verifyhost) { | ^~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:923:5: note: Taking true branch 923 | if(conn_config->verifyhost) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:924:16: note: Calling 'Curl_verify_host' 924 | result = Curl_verify_host(cf, data); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:613:7: note: Assuming 'sspi_status' is equal to SEC_E_OK 613 | if((sspi_status != SEC_E_OK) || !pCertContextServer) { | ^~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:613:6: note: Left side of '||' is false 613 | if((sspi_status != SEC_E_OK) || !pCertContextServer) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:613:35: note: Assuming 'pCertContextServer' is non-null 613 | if((sspi_status != SEC_E_OK) || !pCertContextServer) { | ^~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:613:3: note: Taking false branch 613 | if((sspi_status != SEC_E_OK) || !pCertContextServer) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:622:6: note: Calling 'get_num_host_info' 622 | if(get_num_host_info(p, conn_hostname) || !Win8_compat) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:465:6: note: Assuming 'res' is not equal to 0 465 | if(res) { | ^~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:465:3: note: Taking true branch 465 | if(res) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:466:5: note: The value 4 is assigned to 'ip_blob.size', which participates in a condition later 466 | ip_blob->size = sizeof(struct in_addr); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:622:6: note: Returning from 'get_num_host_info' 622 | if(get_num_host_info(p, conn_hostname) || !Win8_compat) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:622:42: note: Left side of '||' is true 622 | if(get_num_host_info(p, conn_hostname) || !Win8_compat) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:623:9: note: Calling 'get_alt_name_info' 623 | if(!get_alt_name_info(data, pCertContextServer, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 624 | &alt_name_info, &alt_name_info_size)) { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:497:7: note: 'ctx' is non-null 497 | if(!ctx) { | ^~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:497:3: note: Taking false branch 497 | if(!ctx) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:503:6: note: Assuming 'cert_info' is non-null 503 | if(!cert_info) { | ^~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:503:3: note: Taking false branch 503 | if(!cert_info) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:511:6: note: Assuming 'extension' is non-null 511 | if(!extension) { | ^~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:511:3: note: Taking false branch 511 | if(!extension) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:516:7: note: Value assigned to 'alt_name_info', which participates in a condition later 516 | if(!CryptDecodeObjectEx(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 517 | szOID_SUBJECT_ALT_NAME2, | ~~~~~~~~~~~~~~~~~~~~~~~~ 518 | extension->Value.pbData, | ~~~~~~~~~~~~~~~~~~~~~~~~ 519 | extension->Value.cbData, | ~~~~~~~~~~~~~~~~~~~~~~~~ 520 | CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_NOCOPY_FLAG, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 521 | &decode_para, | ~~~~~~~~~~~~~ 522 | alt_name_info, | ~~~~~~~~~~~~~~ 523 | alt_name_info_size)) { | ~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:516:7: note: Value assigned to 'alt_name_info' 516 | if(!CryptDecodeObjectEx(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 517 | szOID_SUBJECT_ALT_NAME2, | ~~~~~~~~~~~~~~~~~~~~~~~~ 518 | extension->Value.pbData, | ~~~~~~~~~~~~~~~~~~~~~~~~ 519 | extension->Value.cbData, | ~~~~~~~~~~~~~~~~~~~~~~~~ 520 | CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_NOCOPY_FLAG, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 521 | &decode_para, | ~~~~~~~~~~~~~ 522 | alt_name_info, | ~~~~~~~~~~~~~~ 523 | alt_name_info_size)) { | ~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:516:6: note: Assuming the condition is false 516 | if(!CryptDecodeObjectEx(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 517 | szOID_SUBJECT_ALT_NAME2, | ~~~~~~~~~~~~~~~~~~~~~~~~ 518 | extension->Value.pbData, | ~~~~~~~~~~~~~~~~~~~~~~~~ 519 | extension->Value.cbData, | ~~~~~~~~~~~~~~~~~~~~~~~~ 520 | CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_NOCOPY_FLAG, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 521 | &decode_para, | ~~~~~~~~~~~~~ 522 | alt_name_info, | ~~~~~~~~~~~~~~ 523 | alt_name_info_size)) { | ~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:516:3: note: Taking false branch 516 | if(!CryptDecodeObjectEx(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:623:9: note: Returning from 'get_alt_name_info' 623 | if(!get_alt_name_info(data, pCertContextServer, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 624 | &alt_name_info, &alt_name_info_size)) { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:623:5: note: Taking false branch 623 | if(!get_alt_name_info(data, pCertContextServer, | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:629:9: note: Field 'size' is 4 629 | if(p->size && alt_name_info) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:629:6: note: Left side of '&&' is true 629 | if(p->size && alt_name_info) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:629:6: note: Assuming pointer value is null 629 | if(p->size && alt_name_info) { | ^~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:629:17: note: Assuming 'alt_name_info' is null 629 | if(p->size && alt_name_info) { | ^~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:629:3: note: Taking false branch 629 | if(p->size && alt_name_info) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:649:41: note: Passing null pointer value via 5th parameter 'alt_name_info' 649 | NULL, 0, alt_name_info, Win8_compat); | ^~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:648:11: note: Calling 'cert_get_name_string' 648 | len = cert_get_name_string(data, pCertContextServer, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 649 | NULL, 0, alt_name_info, Win8_compat); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:384:6: note: Assuming 'Win8_compat' is 0 384 | if(Win8_compat) { | ^~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:384:3: note: Taking false branch 384 | if(Win8_compat) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:402:21: note: 'host_names' is equal to NULL 402 | compute_content = host_names != NULL && length != 0; | ^~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:402:40: note: Left side of '&&' is false 402 | compute_content = host_names != NULL && length != 0; | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:406:6: note: 'compute_content' is 0 406 | if(compute_content) { | ^~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:406:3: note: Taking false branch 406 | if(compute_content) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel_verify.c:413:18: note: Access to field 'cAltEntry' results in a dereference of a null pointer (loaded from variable 'alt_name_info') 413 | for(i = 0; i < alt_name_info->cAltEntry; i++) { | ^~~~~~~~~~~~~ 1 warning generated. ```
https://github.com/curl/curl/actions/runs/13969308325/job/39106970359?pr=16764#step:8:440 ``` /home/runner/work/curl/curl/lib/vtls/schannel.c:986:33: error: Access to field 'sni_hostname' results in a dereference of a null pointer (loaded from field 'cred') [clang-analyzer-core.NullDereference,-warnings-as-errors] 986 | backend->cred->sni_hostname = curlx_convert_UTF8_to_tchar(snihost); | ~~~~ ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:905:3: note: Loop condition is false. Exiting loop 905 | DEBUGASSERT(backend); | ^ /home/runner/work/curl/curl/lib/curl_setup_once.h:301:24: note: expanded from macro 'DEBUGASSERT' 301 | #define DEBUGASSERT(x) do { } while(0) | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:906:3: note: Loop condition is false. Exiting loop 906 | DEBUGF(infof(data, | ^ /home/runner/work/curl/curl/lib/curl_setup_once.h:289:19: note: expanded from macro 'DEBUGF' 289 | #define DEBUGF(x) do { } while(0) | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:910:6: note: Assuming the condition is false 910 | if(curlx_verify_windows_version(5, 1, 0, PLATFORM_WINNT, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 911 | VERSION_LESS_THAN_EQUAL)) { | ~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:910:3: note: Taking false branch 910 | if(curlx_verify_windows_version(5, 1, 0, PLATFORM_WINNT, | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:919:23: note: Assuming field 'alpn' is null 919 | backend->use_alpn = connssl->alpn && s_win_has_alpn; | ^~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:919:37: note: Left side of '&&' is false 919 | backend->use_alpn = connssl->alpn && s_win_has_alpn; | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:934:6: note: Assuming field 'CAfile' is non-null 934 | if(conn_config->CAfile || conn_config->ca_info_blob) { | ^~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:934:26: note: Left side of '||' is true 934 | if(conn_config->CAfile || conn_config->ca_info_blob) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:935:8: note: Assuming the condition is true 935 | if(curlx_verify_windows_version(6, 1, 0, PLATFORM_WINNT, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 936 | VERSION_GREATER_THAN_EQUAL)) { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:935:5: note: Taking true branch 935 | if(curlx_verify_windows_version(6, 1, 0, PLATFORM_WINNT, | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:955:3: note: Null pointer value stored to field 'cred' 955 | backend->cred = NULL; | ^~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:958:6: note: Assuming field 'cache_session' is 0 958 | if(ssl_config->primary.cache_session) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:958:3: note: Taking false branch 958 | if(ssl_config->primary.cache_session) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:975:16: note: Field 'cred' is null 975 | if(!backend->cred) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:975:3: note: Taking true branch 975 | if(!backend->cred) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:978:8: note: Assuming 'result' is 0 978 | if(result) | ^~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:978:5: note: Taking false branch 978 | if(result) | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:985:15: note: Assuming field 'sni' is null 985 | snihost = connssl->peer.sni ? connssl->peer.sni : connssl->peer.hostname; | ^~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:985:15: note: '?' condition is false /home/runner/work/curl/curl/lib/vtls/schannel.c:986:33: note: Access to field 'sni_hostname' results in a dereference of a null pointer (loaded from field 'cred') 986 | backend->cred->sni_hostname = curlx_convert_UTF8_to_tchar(snihost); | ~~~~ ^ ```
Initialize it to CURLE_OK to avoid reading a garbage value in cases. A CURLE_OK is already used one level deeper than necessary in the code, therefore thinking this is correct here. Checkme. Follow-up to 7f4c358 curl#3197 https://github.com/curl/curl/actions/runs/13969308325/job/39106970359?pr=16764#step:8:511 ``` /home/runner/work/curl/curl/lib/vtls/schannel.c:1635:14: error: Assigned value is garbage or undefined [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors] 1635 | result = args.result; | ^ ~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1542:3: note: Loop condition is false. Exiting loop 1542 | DEBUGASSERT(ssl_connect_3 == connssl->connecting_state); | ^ /home/runner/work/curl/curl/lib/curl_setup_once.h:301:24: note: expanded from macro 'DEBUGASSERT' 301 | #define DEBUGASSERT(x) do { } while(0) | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1543:3: note: Loop condition is false. Exiting loop 1543 | DEBUGASSERT(backend); | ^ /home/runner/work/curl/curl/lib/curl_setup_once.h:301:24: note: expanded from macro 'DEBUGASSERT' 301 | #define DEBUGASSERT(x) do { } while(0) | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1545:3: note: Loop condition is false. Exiting loop 1545 | DEBUGF(infof(data, | ^ /home/runner/work/curl/curl/lib/curl_setup_once.h:289:19: note: expanded from macro 'DEBUGF' 289 | #define DEBUGF(x) do { } while(0) | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1549:6: note: Assuming field 'cred' is non-null 1549 | if(!backend->cred) | ^~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1549:3: note: Taking false branch 1549 | if(!backend->cred) | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1553:6: note: Assuming field 'ret_flags' is equal to field 'req_flags' 1553 | if(backend->ret_flags != backend->req_flags) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1553:3: note: Taking false branch 1553 | if(backend->ret_flags != backend->req_flags) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1568:6: note: Assuming field 'use_alpn' is false 1568 | if(backend->use_alpn) { | ^~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1568:3: note: Taking false branch 1568 | if(backend->use_alpn) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1603:6: note: Assuming field 'cache_session' is 0 1603 | if(ssl_config->primary.cache_session) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1603:3: note: Taking false branch 1603 | if(ssl_config->primary.cache_session) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1614:6: note: Assuming field 'certinfo' is not equal to 0 1614 | if(data->set.ssl.certinfo) { | ^~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1614:3: note: Taking true branch 1614 | if(data->set.ssl.certinfo) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1621:9: note: Assuming 'sspi_status' is equal to SEC_E_OK 1621 | if((sspi_status != SEC_E_OK) || !ccert_context) { | ^~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1621:8: note: Left side of '||' is false 1621 | if((sspi_status != SEC_E_OK) || !ccert_context) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1621:37: note: Assuming 'ccert_context' is non-null 1621 | if((sspi_status != SEC_E_OK) || !ccert_context) { | ^~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1621:5: note: Taking false branch 1621 | if((sspi_status != SEC_E_OK) || !ccert_context) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1629:8: note: Assuming 'result' is 0 1629 | if(!result) { | ^~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1629:5: note: Taking true branch 1629 | if(!result) { | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1634:7: note: Calling 'traverse_cert_store' 1634 | traverse_cert_store(ccert_context, add_cert_to_certinfo, &args); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1452:9: note: 'should_continue' is true 1452 | while(should_continue && | ^~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1452:9: note: Left side of '&&' is true /home/runner/work/curl/curl/lib/vtls/schannel.c:1453:9: note: Assuming the condition is false 1453 | (current_context = CertEnumCertificatesInStore( | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1454 | context->hCertStore, | ~~~~~~~~~~~~~~~~~~~~ 1455 | current_context)) != NULL) { | ~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1452:3: note: Loop condition is false. Execution continues on line 1467 1452 | while(should_continue && | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1467:6: note: 'current_context' is null 1467 | if(current_context) | ^~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1467:3: note: Taking false branch 1467 | if(current_context) | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1469:1: note: Returning without writing to 'arg->result' 1469 | } | ^ /home/runner/work/curl/curl/lib/vtls/schannel.c:1634:7: note: Returning from 'traverse_cert_store' 1634 | traverse_cert_store(ccert_context, add_cert_to_certinfo, &args); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1635:14: note: Assigned value is garbage or undefined 1635 | result = args.result; | ^ ~~~~~~~~~~~ ```
https://github.com/curl/curl/actions/runs/13969308325/job/39106970359?pr=16764#step:8:612 ``` /home/runner/work/curl/curl/lib/vtls/schannel.c:1944:7: error: Value stored to 'nread' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors] 1944 | nread = -1; | ^ ~~ /home/runner/work/curl/curl/lib/vtls/schannel.c:1944:7: note: Value stored to 'nread' is never read 1944 | nread = -1; | ^ ~~ ```
https://github.com/curl/curl/actions/runs/13972569875/job/39118035675?pr=16764#step:8:214 ``` home/runner/work/curl/curl/src/tool_doswin.c:180:7: error: Value stored to 'len' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors] 180 | len = clip - target; | ^ ~~~~~~~~~~~~~ 1 warning generated. ```
```
/home/runner/work/curl/curl/tests/libtest/testutil.c:157:12: error: Potential leak of memory pointed to by 'path' [clang-analyzer-unix.Malloc,-warnings-as-errors]
157 | return NULL;
| ^
```
https://github.com/curl/curl/actions/runs/13972861386/job/39119020171?pr=16764#step:10:269
Not detected by clang-tidy.
2 tasks
vszakats
commented
Mar 20, 2025
| @@ -1631,6 +1631,7 @@ schannel_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data) | |||
| args.data = data; | |||
| args.idx = 0; | |||
| args.certs_count = certs_count; | |||
| args.result = CURLE_OK; | |||
There was a problem hiding this comment.
I don't understand this code, so I'm not sure about this one.
At a lower level, the same default result is set. The issue is that it's one level too low, and a call above it misses it an leaves it uninitialized. So I set it here at the most upper level as a fix. But, defaulting to OK in a security setting feels a bit uncomfortable.
vszakats
added a commit
that referenced
this pull request
Mar 24, 2025
- linux: wolfssl, wolfssh (replacing libssh2), ech, kerberos/GSSAPI, ssls-export (libssh2 remains tested on macos.) - macos: brotli, zstd, c-ares, gnutls, mbedtls, gsasl, rtmp, ssls-export - windows: new job with schannel, sspi, winidn, winldap, ssls-export - unit3205: fix/silence remaining NULL dereferences. Commits fixing the issues found: cbbccb8 #16766 554e4c1 #16777 Closes #16764
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
calloc()fail.(not detected by clang-tidy)
NULLalt_name_info.NULLderef forbackend→cred.Follow-up to 7f4c358 Schannel CURLOPT_CERTINFO support. #3197
(not detected by clang-tidy)
Cherry-picked from #16764