On Tue, Nov 05, 2019 at 11:01:56AM -0300, Alexandre Hamada wrote:
> Hi Claudio,
>
> I was testing some tal files, and when it contains an https url (RFC 8630),
> it generates a segmentation fault.
>
> Thus, I would like to suggest adding the following on tal_parse/tal_free.
>
> Example:
> https://rir.docker/ta/ta.cer
>
> MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAoFVWio0auchdC1mCbp4N
> pSG9b43tXDIIOHHsc/6fTg0wdFdX68qHFYAwTlXVriXs5j0WmB8DZ1qiE4nyYfEj
> kVIU6bVbz8WZ03BpajGO1AfbasBdbAr4SWjl8YCyCFjfojij2SnH/lbjxca5mSa0
> wJAT4LEG/B4kMROcKRSebTCi8iDxMYha1GFFXBD9ze9qpEoUE3ZobQfITMNd8xZx
> mZyg6bXXnT8qj7m5IrCs7dD4qrhjPTKbxwlpXFlS4pNdh9p9H/gFHxF7SbOLIq4m
> 2rBQKNCMzvFnFgWJyM2QqiCb89WJf6kvyBjMbh7h/HygaXy8SuXUprExudQy8OZp
> xwIDAQAB
>
>
> Regards,
> Alexandre Hamada
>
> @@ -142,18 +142,19 @@ tal_parse(const char *fn, char *buf)
>
> p = tal_parse_buffer(fn, buf);
>
> - /* extract the TAL basename (without .tal suffix) */
> - d = basename((char*)fn);
> - if (d == NULL)
> - err(EXIT_FAILURE, "%s: basename", fn);
> - dlen = strlen(d);
> - if (strcasecmp(d + dlen - 4, ".tal") == 0)
> - dlen -= 4;
> - if ((p->descr = malloc(dlen + 1)) == NULL)
> - err(EXIT_FAILURE, NULL);
> - memcpy(p->descr, d, dlen);
> - p->descr[dlen] = 0;
> -
> + if(p != NULL) {
> + /* extract the TAL basename (without .tal suffix) */
> + d = basename((char*)fn);
> + if (d == NULL)
> + err(EXIT_FAILURE, "%s: basename", fn);
> + dlen = strlen(d);
> + if (strcasecmp(d + dlen - 4, ".tal") == 0)
> + dlen -= 4;
> + if ((p->descr = malloc(dlen + 1)) == NULL)
> + err(EXIT_FAILURE, NULL);
> + memcpy(p->descr, d, dlen);
> + p->descr[dlen] = 0;
> + }
> return p;
> }
>
I decided to just check p after calling tal_parse_buffer() for NULL and
return early in that case.
> @@ -231,9 +232,15 @@ tal_free(struct tal *p)
> for (i = 0; i < p->urisz; i++)
> free(p->uri[i]);
>
> - free(p->pkey);
> - free(p->uri);
> - free(p->descr);
> + if (p->pkey != NULL) {
> + free(p->pkey);
> + }
> + if (p->uri != NULL) {
> + free(p->uri);
> + }
> + if (p->descr != NULL) {
> + free(p->descr);
> + }
> free(p);
> }
>
There is no need to check a pointer for NULL in free(). free() handles
NULL just fine so it is considered bad style to do if (foo != NULL) free(foo);
Thanks for the report.
--
:wq Claudio
