crypto/asn1/a_string.c: null pointer dereference suspected by gcc12 static analyzer

[chipitsine]

/home/ilia/temp/libressl-portable/crypto/asn1/a_string.c:373:34: warning: dereference of NULL ‘s’ [CWE-476] [-Wanalyzer-null-dereference]
  373 |                                 s[num + j] <<= 4;
      |                                 ~^~~~~~~~~
  ‘a2i_ASN1_STRING’: events 1-11
    |
    |  317 |                 if (bufsize < 1) {
    |      |                    ^
    |      |                    |
    |      |                    (1) following ‘false’ branch (when ‘bufsize > 0’)...
    |......
    |  326 |                 if (buf[i-1] == '\n')
    |      |                        ~
    |      |                        |
    |      |                        (2) ...to here
    |  327 |                         buf[--i] = '\0';
    |  328 |                 if (i == 0)
    |      |                    ~
    |      |                    |
    |      |                    (3) following ‘false’ branch (when ‘i != 0’)...
    |  329 |                         goto err_sl;
    |  330 |                 if (buf[i-1] == '\r')
    |      |                        ~
    |      |                        |
    |      |                        (4) ...to here
    |  331 |                         buf[--i] = '\0';
    |  332 |                 if (i == 0)
    |      |                    ~
    |      |                    |
    |      |                    (5) following ‘false’ branch (when ‘i != 0’)...
    |  333 |                         goto err_sl;
    |  334 |                 if (buf[i - 1] == '\\') {
    |      |                        ~
    |      |                        |
    |      |                        (6) ...to here
    |......
    |  340 |                 if (i < 2)
    |      |                    ~
    |      |                    |
    |      |                    (7) following ‘false’ branch (when ‘i > 1’)...
    |......
    |  346 |                 if (i % 2 != 0) {
    |      |                    ~
    |      |                    |
    |      |                    (8) ...to here
    |      |                    (9) following ‘false’ branch...
    |......
    |  350 |                 i /= 2;
    |      |                 ~~~~~~
    |      |                   |
    |      |                   (10) ...to here
    |  351 |                 if (num + i > slen) {
    |      |                    ~
    |      |                    |
    |      |                    (11) following ‘false’ branch...
    |
  ‘a2i_ASN1_STRING’: event 12
    |
    |cc1:
    | (12): ...to here
    |
  ‘a2i_ASN1_STRING’: event 13
    |
    |  360 |                 for (j = 0; j < i; j++, k += 2) {
    |      |                             ~~^~~
    |      |                               |
    |      |                               (13) following ‘true’ branch (when ‘j < i’)...
    |
  ‘a2i_ASN1_STRING’: event 14
    |
    |cc1:
    | (14): ...to here
    |
  ‘a2i_ASN1_STRING’: events 15-18
    |
    |  361 |                         for (n = 0; n < 2; n++) {
    |      |                                     ~~^~~
    |      |                                       |
    |      |                                       (15) following ‘true’ branch (when ‘n != 2’)...
    |  362 |                                 m = bufp[k + n];
    |      |                                          ~~~~~
    |      |                                            |
    |      |                                            (16) ...to here
    |......
    |  373 |                                 s[num + j] <<= 4;
    |      |                                 ~~~~~~~~~~
    |      |                                  |
    |      |                                  (17) ‘s’ is NULL
    |      |                                  (18) dereference of NULL ‘s + ((long unsigned int)j + num)’
    |

[botovq]

I think this is a false positive.

|  351 |                 if (num + i > slen) {

https://github.com/libressl-portable/openbsd/blob/9b1a3fbf691c04bbcd9bcde6260cdc1991547af0/src/lib/libcrypto/asn1/a_string.c#L351

The first time this conditional is reached, num == 0 and slen == 0 while i >= 1. So s is allocated by realloc() with a size > 0, so it is never NULL if this point is reached:

|......
|  373 |                                 s[num + j] <<= 4;
|      |                                 ~~~~~~~~~~
|      |                                  |
|      |                                  (17) ‘s’ is NULL

In other words, I don't see how this can happen.

[chipitsine]

thanks, I'll report it to gcc

[chipitsine]

can we put

		for (j = 0; j < i; j++, k += 2) {
			for (n = 0; n < 2; n++) {

inside

   		if (num + i > slen) {
			sp = realloc(s, num + i);
			if (sp == NULL) {
				ASN1error(ERR_R_MALLOC_FAILURE);
				goto err;
			}
			s = sp;
			slen = num + i;
		}

I do not see possibility for static analyzer to determine that s is not null after it suspects if (num + i > slen) { false