I ran analyzer, it shows old findings. am I missing something? or patches
were not yet applied
diff =
diff -u -p a/crypto/asn1/bio_ndef.c b/crypto/asn1/bio_ndef.c
--- a/crypto/asn1/bio_ndef.c
+++ b/crypto/asn1/bio_ndef.c
@@ -181,6 +181,8 @@ ndef_prefix(BIO *b, unsigned char **pbuf
derlen = ASN1_item_ndef_i2d(ndef_aux->val, NULL, ndef_aux->it);
p = malloc(derlen);
+ if (p == NULL)
+ return;
ndef_aux->derbuf = p;
*pbuf = p;
derlen = ASN1_item_ndef_i2d(ndef_aux->val, &p, ndef_aux->it);
@@ -253,6 +255,8 @@ ndef_suffix(BIO *b, unsigned char **pbuf
derlen = ASN1_item_ndef_i2d(ndef_aux->val, NULL, ndef_aux->it);
p = malloc(derlen);
+ if (p == NULL)
+ return;
ndef_aux->derbuf = p;
*pbuf = p;
derlen = ASN1_item_ndef_i2d(ndef_aux->val, &p, ndef_aux->it);
Skipping: ./crypto/asn1/p5_pbe.c
HANDLING: ./ssl/d1_pkt.c
diff =
diff -u -p a/ssl/d1_pkt.c b/ssl/d1_pkt.c
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -214,6 +214,8 @@ dtls1_buffer_record(SSL *s, record_pqueu
return 0;
rdata = malloc(sizeof(DTLS1_RECORD_DATA_INTERNAL));
+ if (rdata == NULL)
+ return;
item = pitem_new(priority, rdata);
if (rdata == NULL || item == NULL)
goto init_err;
@@ -260,6 +262,8 @@ dtls1_buffer_rcontent(SSL *s, rcontent_p
return 0;
rdata = malloc(sizeof(DTLS1_RCONTENT_DATA_INTERNAL));
+ if (rdata == NULL)
+ return;
item = pitem_new(priority, rdata);
if (rdata == NULL || item == NULL)
goto init_err;
Skipping: ./ssl/d1_srtp.c
Skipping: ./apps/openssl/compat/clock_gettime_osx.c
HANDLING: ./apps/openssl/apps.c
diff =
diff -u -p a/apps/openssl/apps.c b/apps/openssl/apps.c
--- a/apps/openssl/apps.c
+++ b/apps/openssl/apps.c
@@ -310,6 +310,8 @@ password_callback(char *buf, int bufsiz,
PW_MIN_LENGTH, bufsiz - 1);
if (ok >= 0 && verify) {
buff = malloc(bufsiz);
+ if (buff == NULL)
+ return;
ok = UI_add_verify_string(ui, prompt, ui_flags, buff,
PW_MIN_LENGTH, bufsiz - 1, buf);
}
Skipping: ./apps/openssl/asn1pars.c
вт, 16 мая 2023 г. в 21:18, Theo Buehler <t...@theobuehler.org>:
> > I tried to find "missing malloc null check" using the following
> coccinelle
> > script (easy to run from within CI)
>
> Cool, that's nice. We tend to be strict with error checking in new code,
> but having such a sanity check certainly won't hurt. If we only need to
> fix half a dozen functions, it might actually be worth it. Thanks
>
> > diff -u -p a/apps/openssl/apps.c b/apps/openssl/apps.c
> > --- a/apps/openssl/apps.c
> > +++ b/apps/openssl/apps.c
> > @@ -379,6 +379,8 @@ password_callback(char *buf, int bufsiz,
> > PW_MIN_LENGTH, bufsiz - 1);
> > if (ok >= 0 && verify) {
> > buff = malloc(bufsiz);
> > +if (buff == NULL)
> > +return;
> > ok = UI_add_verify_string(ui, prompt, ui_flags, buff,
> > PW_MIN_LENGTH, bufsiz - 1, buf);
>
> UI_add_verify_string() handles a NULL buff correctly, although it is
> far from obvious. We will end up in general_allocate_prompt() with
> type == UIT_VERIFY and result_buf == NULL and will return -1 eventually.
>
> If we wanted to NULL check the allocation here, we'd need to rework this
> entire function (which we should probably do anyway).
>
> > diff -u -p a/crypto/asn1/bio_ndef.c b/crypto/asn1/bio_ndef.c
> > --- a/crypto/asn1/bio_ndef.c
> > +++ b/crypto/asn1/bio_ndef.c
>
> Already discussed.
>
> > diff -u -p a/tests/ecdhtest.c b/tests/ecdhtest.c
> > --- a/tests/ecdhtest.c
> > +++ b/tests/ecdhtest.c
>
> Thanks, I committed an appropriate version of this.
>
> > diff -u -p a/ssl/d1_pkt.c b/ssl/d1_pkt.c
> > --- a/ssl/d1_pkt.c
> > +++ b/ssl/d1_pkt.c
> > @@ -214,6 +214,8 @@ dtls1_buffer_record(SSL *s, record_pqueu
> > return 0;
> >
> > rdata = malloc(sizeof(DTLS1_RECORD_DATA_INTERNAL));
> > +if (rdata == NULL)
> > +return;
> > item = pitem_new(priority, rdata);
> > if (rdata == NULL || item == NULL)
> > goto init_err;
>
> The checks actually happen in 'if (rdata == NULL || item == NULL)' above.
> pitem_new() does not dereference rdata, so the error checks are fine as
> they are.
>
> We could rewrite this to do the following, with a corresponding
> NULL-initialization of item at the top of the function. I would actually
> prefer this.
>
> if ((rdata = malloc(sizeof(*rdata))) == NULL)
> goto init_err;
> if ((item = pitem_new(priority, rdata)) == NULL)
> goto init_err;
>
> > @@ -260,6 +262,8 @@ dtls1_buffer_rcontent(SSL *s, rcontent_p
> > return 0;
> >
> > rdata = malloc(sizeof(DTLS1_RCONTENT_DATA_INTERNAL));
> > +if (rdata == NULL)
> > +return;
> > item = pitem_new(priority, rdata);
> > if (rdata == NULL || item == NULL)
> > goto init_err;
> >
>
> Exactly the same here.
>
