Skip to content

Fix -4 return code when expected BAD_FUNC_ARG(-173)#6842

Merged
SparkiDev merged 2 commits intowolfSSL:masterfrom
kaleb-himes:fix-err-introduce-with-cm-move
Oct 8, 2023
Merged

Fix -4 return code when expected BAD_FUNC_ARG(-173)#6842
SparkiDev merged 2 commits intowolfSSL:masterfrom
kaleb-himes:fix-err-introduce-with-cm-move

Conversation

@kaleb-himes
Copy link
Copy Markdown
Contributor

Description

A PR had introduced a specific error when building with configuration --enable-trackmemory --enable-smallstack CFLAGS="-DALT_ECC_SIZE" some code in ssl_certman.c was over-writing an expected error value -173 (BAD_FUNC_ARG) with the error code WOLFSSL_BAD_FILE (-4) causing api.c to fail an expected BAD_FUNC_ARG check.

Testing

Using the above configuration with commit: 3af87f6 that introduced the error and running ./tests/unit.test

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@kaleb-himes kaleb-himes requested a review from SparkiDev October 5, 2023 20:36
@kaleb-himes kaleb-himes self-assigned this Oct 5, 2023
Comment thread src/ssl_certman.c Outdated
ret = WOLFSSL_BAD_FILE;
/* If some other error has already occurred preserve that code
* return WOLFSSL_BAD_FILE if that is the only error condition */
if (ret == WOLFSSL_SUCCESS) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hang on. Why are we not enabling the check of ret above instead of doing the malloc when we are already failing.

Copy link
Copy Markdown
Contributor Author

@kaleb-himes kaleb-himes Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can go with that approach instead and do the initialization to satisfy the compiler:

diff --git a/src/ssl_certman.c b/src/ssl_certman.c
index 9f227be15..46011595f 100644
--- a/src/ssl_certman.c
+++ b/src/ssl_certman.c
@@ -782,7 +782,7 @@ int wolfSSL_CertManagerVerify(WOLFSSL_CERT_MANAGER* cm, const char* fname,
 #ifndef WOLFSSL_SMALL_STACK
     byte   staticBuffer[FILE_BUFFER_SIZE];
 #endif
-    byte*  buff;
+    byte*  buff = NULL;
     long   sz = 0;
     XFILE  file = XBADFILE;
 
@@ -814,16 +814,15 @@ int wolfSSL_CertManagerVerify(WOLFSSL_CERT_MANAGER* cm, const char* fname,
      * small. */
 #ifndef WOLFSSL_SMALL_STACK
     if ((ret == WOLFSSL_SUCCESS) && (sz > (long)sizeof(staticBuffer)))
+#else
+
+    if (ret == WOLFSSL_SUCCESS)
 #endif
     {
         WOLFSSL_MSG("Getting dynamic buffer");
         buff = (byte*)XMALLOC((size_t)sz, cm->heap, DYNAMIC_TYPE_FILE);
         if (buff == NULL) {
-            /* If some other error has already occurred preserve that code
-             * return WOLFSSL_BAD_FILE if that is the only error condition */
-            if (ret == WOLFSSL_SUCCESS) {
-                ret = WOLFSSL_BAD_FILE;
-            }
+            ret = WOLFSSL_BAD_FILE;
         }
     }
     /* Read all the file into buffer. */

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@JacobBarthelmeh
Copy link
Copy Markdown
Contributor

retest this please jenkins

@SparkiDev SparkiDev merged commit 832e0f3 into wolfSSL:master Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants