heimdal
heimdal copied to clipboard
heimdal
Invalid free() generated from asn1_compile in not-template mode
Again, current Heimdal imported into Samba and with the crmf code compiled (I'll try and just use the header, but our build system likes depending on the whole ball of wax).
$ head -n 2 /etc/os-release
NAME=Fedora
VERSION="34 (Container Image)"
$ gcc -v
...
gcc version 11.1.1 20210428 (Red Hat 11.1.1-1) (GCC)
The compiler is noticing that p becomes undefined (per the C standard I'm told). That is, if p + 1 is a valid pointer, then p must point before the area allocated by malloc(), and so operations on that point, while sensible, are undefined. As you know our C compiler authors are keen to use every excuse of undefined behaviour against old code such as Heimdal and Samba...
@josephsutton1 pointed me at the code that detects this: https://github.com/gcc-mirror/gcc/blob/a3543b5e8002c033b2304d7ac1d1e58218eebb51/gcc/builtins.c#L13863 and tells me that "Addition or subtraction of a pointer into, or just beyond, an array object and an integer type [producing] a result that does not point into, or just beyond, the same array object" is undefined behaviour
source4/heimdal/lib/asn1/asn1_crmf_asn1.c: In function ‘encode_POPOSigningKeyInput’:
source4/heimdal/lib/asn1/asn1_crmf_asn1.c:453:1: error: ‘free’ called on pointer ‘p’ with nonzero offset [1, 9223372036854775807] [-Werror=free-nonheap-object]
453 | free(p + 1);
| ^~~~~~~~~~~
source4/heimdal/lib/asn1/asn1_crmf_asn1.c:439:10: note: returned from ‘malloc’
439 | if ((p = malloc(len)) == NULL) return ENOMEM;
| ^~~~~~~~~~~
While the generated code does check the value of p comes back to the malloc() value, and assert()s otherwise, asn1_compile should generate code that saves p first and free()s that instead.
@nicowilliams last touched the ASN.1 compiler, if he doesn't have time I can take a look though
I can't find the assert you mentioned?
What if asn1_compile incremented p immediately prior to calling free(), would that squash the warning?
No, I don't think that would be enough. It notices if p had ever referred to a memory location before the return from malloc() or the argument to free() as I understand it. Once undefined the compiler then has licence to do what it wants, per the above.
To reproduce you need to use Samba's full set of compiler warnings, or presumably at least -Werror=free-nonheap-object
