Hey,
Adding Patrick...
On Mon, Feb 05, 2024 at 03:39:33PM +0800, Gary Lin wrote:
> GIT repo for v9: https://github.com/lcp/grub2/tree/tpm2-unlock-v9
>
> This patch series is based on "Automatic TPM Disk Unlock"(*1) posted by
> Hernan Gatta to introduce the key protector framework and TPM2 stack
> to GRUB2, and this could be a useful feature for the systems to
> implement full disk encryption.
Sadly this patch set have many issues...
The git complains in the following way...
Applying: asn1_test: test module for libtasn1
.git/rebase-apply/patch:1374: new blank line at EOF.
warning: 1 line adds whitespace errors.
Applying: libtasn1: Add the documentation
.git/rebase-apply/patch:90: trailing whitespace.
.git/rebase-apply/patch:92: trailing whitespace.
.git/rebase-apply/patch:99: trailing whitespace.
.git/rebase-apply/patch:102: trailing whitespace.
.git/rebase-apply/patch:108: trailing whitespace.
warning: squelched 80 whitespace errors
warning: 85 lines add whitespace errors.
The developers manual does not build due to following errors...
grub-dev.texi:616: misplaced {
grub-dev.texi:616: misplaced }
grub-dev.texi:617: misplaced {
grub-dev.texi:617: misplaced }
grub-dev.texi:580: warning: node `libtasn1' is next for `minilzo' in
sectioning but not in menu
grub-dev.texi:599: warning: unreferenced node `libtasn1'
grub-dev.texi:599: warning: node `minilzo' is prev for `libtasn1' in
sectioning but not in menu
grub-dev.texi:599: warning: node `Updating External Code' is up for
`libtasn1' in sectioning but not in menu
grub-dev.texi:499: node `Updating External Code' lacks menu item for
`libtasn1' despite being its Up target
And I have attached the Coverity report. All issues reported there have
to be fixed. If you cannot fix an issue you have to explain why you
cannot do that and what is potential impact on the code stability,
security, etc.
Please do not forget to check code which you add adhere to GRUB coding
style [1]. Good example is in grub-core/kern/efi/sb.c too. Of course
this requirement does not apply to the libs which you import.
Additionally, please CC patrick.c...@oracle.com next time. He is
interested in this patch set development.
Daniel
[1] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Coding-style
Hi,
Please find the latest report on new defect(s) introduced to GRUB found with
Coverity Scan.
15 new defect(s) introduced to GRUB found with Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 15 of 15 defect(s)
** CID 435775: Resource leaks (RESOURCE_LEAK)
/util/grub-protect.c: 982 in grub_protect_tpm2_add()
________________________________________________________________________________________________________
*** CID 435775: Resource leaks (RESOURCE_LEAK)
/util/grub-protect.c: 982 in grub_protect_tpm2_add()
976
977 if (key_size > TPM_MAX_SYM_DATA)
978 {
979 fprintf (stderr,
980 _("Input key is too long, maximum allowed size is %u
bytes.\n"),
981 TPM_MAX_SYM_DATA);
>>> CID 435775: Resource leaks (RESOURCE_LEAK)
>>> Variable "key" going out of scope leaks the storage it points to.
982 return GRUB_ERR_OUT_OF_RANGE;
983 }
984
985 err = grub_protect_tpm2_get_srk (args, &srk);
986 if (err != GRUB_ERR_NONE)
987 goto exit2;
** CID 435774: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
/grub-core/lib/libtasn1/lib/decoding.c: 481 in asn1_get_object_id_der()
________________________________________________________________________________________________________
*** CID 435774: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
/grub-core/lib/libtasn1/lib/decoding.c: 481 in asn1_get_object_id_der()
475 */
476 if (leading != 0 && der[len_len + k] == 0x80)
477 return ASN1_DER_ERROR;
478 leading = 0;
479
480 /* check for wrap around */
>>> CID 435774: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
>>> "val < ((((1 ? 0 : val) - 1 < 0) ? ~((((1 ? 0 : val) + 1 << 62UL /*
>>> sizeof (+val) * 8 - 2 */) - 1) * 2 + 1) : ((1 ? 0 : val) + 0)) >> 7)" is
>>> always false regardless of the values of its operands. This occurs as the
>>> second operand of "?:".
481 if (INT_LEFT_SHIFT_OVERFLOW (val, 7))
482 return ASN1_DER_ERROR;
483
484 val = val << 7;
485 val |= der[len_len + k] & 0x7F;
486
** CID 435773: Integer handling issues (NO_EFFECT)
/grub-core/lib/libtasn1/lib/decoding.c: 439 in asn1_get_object_id_der()
________________________________________________________________________________________________________
*** CID 435773: Integer handling issues (NO_EFFECT)
/grub-core/lib/libtasn1/lib/decoding.c: 439 in asn1_get_object_id_der()
433 return ASN1_DER_ERROR;
434
435 val0 = 0;
436
437 for (k = 0; k < len; k++)
438 {
>>> CID 435773: Integer handling issues (NO_EFFECT)
>>> This less-than-zero comparison of an unsigned value is never true. "(1
>>> ? 0UL : val0) - 1UL < 0UL".
439 if (INT_LEFT_SHIFT_OVERFLOW (val0, 7))
440 return ASN1_DER_ERROR;
441
442 val0 <<= 7;
443 val0 |= der[len_len + k] & 0x7F;
444 if (!(der[len_len + k] & 0x80))
** CID 435772: Integer handling issues (NO_EFFECT)
/grub-core/lib/libtasn1/lib/decoding.c: 204 in asn1_get_tag_der()
________________________________________________________________________________________________________
*** CID 435772: Integer handling issues (NO_EFFECT)
/grub-core/lib/libtasn1/lib/decoding.c: 204 in asn1_get_tag_der()
198 /* Long form */
199 punt = 1;
200 ris = 0;
201 while (punt < der_len && der[punt] & 128)
202 {
203
>>> CID 435772: Integer handling issues (NO_EFFECT)
>>> This less-than-zero comparison of an unsigned value is never true. "(1
>>> ? 0U : ((1 ? 0U : ris) + 128U)) - 1U < 0U".
204 if (INT_MULTIPLY_OVERFLOW (ris, 128))
205 return ASN1_DER_ERROR;
206 ris *= 128;
207
208 if (INT_ADD_OVERFLOW (ris, ((unsigned) (der[punt] & 0x7F))))
209 return ASN1_DER_ERROR;
** CID 435771: (RESOURCE_LEAK)
/util/grub-protect.c: 982 in grub_protect_tpm2_add()
/util/grub-protect.c: 971 in grub_protect_tpm2_add()
/util/grub-protect.c: 1027 in grub_protect_tpm2_add()
________________________________________________________________________________________________________
*** CID 435771: (RESOURCE_LEAK)
/util/grub-protect.c: 982 in grub_protect_tpm2_add()
976
977 if (key_size > TPM_MAX_SYM_DATA)
978 {
979 fprintf (stderr,
980 _("Input key is too long, maximum allowed size is %u
bytes.\n"),
981 TPM_MAX_SYM_DATA);
>>> CID 435771: (RESOURCE_LEAK)
>>> Variable "grub_drive" going out of scope leaks the storage it points to.
982 return GRUB_ERR_OUT_OF_RANGE;
983 }
984
985 err = grub_protect_tpm2_get_srk (args, &srk);
986 if (err != GRUB_ERR_NONE)
987 goto exit2;
/util/grub-protect.c: 971 in grub_protect_tpm2_add()
965 char *grub_drive = NULL;
966
967 grub_protect_get_grub_drive_for_file (args->tpm2_outfile,
&grub_drive);
968
969 err = grub_protect_tpm2_open_device (args->tpm2_device);
970 if (err != GRUB_ERR_NONE)
>>> CID 435771: (RESOURCE_LEAK)
>>> Variable "grub_drive" going out of scope leaks the storage it points to.
971 return err;
972
973 err = grub_protect_read_file (args->tpm2_keyfile, (void **)&key,
&key_size);
974 if (err != GRUB_ERR_NONE)
975 goto exit1;
976
/util/grub-protect.c: 1027 in grub_protect_tpm2_add()
1021 exit2:
1022 grub_free (key);
1023
1024 exit1:
1025 grub_protect_tpm2_close_device ();
1026
>>> CID 435771: (RESOURCE_LEAK)
>>> Variable "grub_drive" going out of scope leaks the storage it points to.
1027 return err;
1028 }
1029
1030 static grub_err_t
1031 grub_protect_tpm2_remove (struct grub_protect_args *args)
1032 {
** CID 435770: Insecure data handling (TAINTED_SCALAR)
/grub-core/tpm2/mu.c: 564 in grub_tpm2_mu_TPM2B_Unmarshal()
________________________________________________________________________________________________________
*** CID 435770: Insecure data handling (TAINTED_SCALAR)
/grub-core/tpm2/mu.c: 564 in grub_tpm2_mu_TPM2B_Unmarshal()
558 void
559 grub_tpm2_mu_TPM2B_Unmarshal (grub_tpm2_buffer_t buffer,
560 TPM2B* p)
561 {
562 grub_tpm2_buffer_unpack_u16 (buffer, &p->size);
563
>>> CID 435770: Insecure data handling (TAINTED_SCALAR)
>>> Using tainted variable "p->size" as a loop boundary.
564 for (grub_uint16_t i = 0; i < p->size; i++)
565 grub_tpm2_buffer_unpack_u8 (buffer, &p->buffer[i]);
566 }
567
568 void
569 grub_tpm2_mu_TPMS_AUTH_RESPONSE_Unmarshal (grub_tpm2_buffer_t buffer,
** CID 435769: Null pointer dereferences (FORWARD_NULL)
________________________________________________________________________________________________________
*** CID 435769: Null pointer dereferences (FORWARD_NULL)
/grub-core/tpm2/module.c: 222 in grub_tpm2_protector_srk_read_file()
216 *buffer = read_buffer;
217 *buffer_size = file_size;
218
219 err = GRUB_ERR_NONE;
220
221 error:
>>> CID 435769: Null pointer dereferences (FORWARD_NULL)
>>> Passing null pointer "file" to "grub_file_close", which dereferences it.
222 grub_file_close (file);
223
224 return err;
225 }
226
227 static grub_err_t
** CID 435768: (CONSTANT_EXPRESSION_RESULT)
/grub-core/lib/libtasn1/lib/decoding.c: 204 in asn1_get_tag_der()
/grub-core/lib/libtasn1/lib/decoding.c: 217 in asn1_get_tag_der()
________________________________________________________________________________________________________
*** CID 435768: (CONSTANT_EXPRESSION_RESULT)
/grub-core/lib/libtasn1/lib/decoding.c: 204 in asn1_get_tag_der()
198 /* Long form */
199 punt = 1;
200 ris = 0;
201 while (punt < der_len && der[punt] & 128)
202 {
203
>>> CID 435768: (CONSTANT_EXPRESSION_RESULT)
>>> "ris < (((1 ? 0 : ((1 ? 0 : ris) + 128)) - 1 < 0) ? ~((((1 ? 0 : ((1 ?
>>> 0 : ris) + 128)) + 1 << 30UL /* sizeof (+((1 ? 0 : ris) + 128)) * 8 - 2 */)
>>> - 1) * 2 + 1) : ((1 ? 0 : ((1 ? 0 : ris) + 128)) + 0)) / 128" is always
>>> false regardless of the values of its operands. This occurs as the second
>>> operand of "?:".
204 if (INT_MULTIPLY_OVERFLOW (ris, 128))
205 return ASN1_DER_ERROR;
206 ris *= 128;
207
208 if (INT_ADD_OVERFLOW (ris, ((unsigned) (der[punt] & 0x7F))))
209 return ASN1_DER_ERROR;
/grub-core/lib/libtasn1/lib/decoding.c: 217 in asn1_get_tag_der()
211 punt++;
212 }
213
214 if (punt >= der_len)
215 return ASN1_DER_ERROR;
216
>>> CID 435768: (CONSTANT_EXPRESSION_RESULT)
>>> "ris < (((1 ? 0 : ((1 ? 0 : ris) + 128)) - 1 < 0) ? ~((((1 ? 0 : ((1 ?
>>> 0 : ris) + 128)) + 1 << 30UL /* sizeof (+((1 ? 0 : ris) + 128)) * 8 - 2 */)
>>> - 1) * 2 + 1) : ((1 ? 0 : ((1 ? 0 : ris) + 128)) + 0)) / 128" is always
>>> false regardless of the values of its operands. This occurs as the second
>>> operand of "?:".
217 if (INT_MULTIPLY_OVERFLOW (ris, 128))
218 return ASN1_DER_ERROR;
219 ris *= 128;
220
221 if (INT_ADD_OVERFLOW (ris, ((unsigned) (der[punt] & 0x7F))))
222 return ASN1_DER_ERROR;
** CID 435767: Insecure data handling (TAINTED_SCALAR)
________________________________________________________________________________________________________
*** CID 435767: Insecure data handling (TAINTED_SCALAR)
/grub-core/tpm2/mu.c: 974 in grub_tpm2_mu_TPML_DIGEST_Unmarshal()
968 grub_tpm2_mu_TPML_DIGEST_Unmarshal (grub_tpm2_buffer_t buf,
969 TPML_DIGEST* digest)
970 {
971 grub_tpm2_buffer_unpack_u32 (buf, &digest->count);
972
973 for (grub_uint32_t i = 0; i < digest->count; i++)
>>> CID 435767: Insecure data handling (TAINTED_SCALAR)
>>> Passing tainted expression "digest->digests[i]->size" to
>>> "grub_tpm2_mu_TPM2B_DIGEST_Unmarshal", which uses it as a loop boundary.
974 grub_tpm2_mu_TPM2B_DIGEST_Unmarshal (buf, &digest->digests[i]);
975 }
976
977 void
978 grub_tpm2_mu_TPMS_SIGNATURE_RSA_Unmarshal (grub_tpm2_buffer_t buffer,
979 TPMS_SIGNATURE_RSA *rsa)
** CID 435766: Memory - corruptions (OVERRUN)
________________________________________________________________________________________________________
*** CID 435766: Memory - corruptions (OVERRUN)
/grub-core/lib/libtasn1/lib/decoding.c: 1204 in asn1_der_decoding2()
1198 }
1199
1200 DECR_LEN (ider_len, len2);
1201
1202 tlen = strlen (temp);
1203 if (tlen > 0)
>>> CID 435766: Memory - corruptions (OVERRUN)
>>> Allocating insufficient memory for the terminating null of the string.
1204 _asn1_set_value (p, temp, tlen);
1205
1206 counter += len2;
1207 move = RIGHT;
1208 break;
1209 case ASN1_ETYPE_OCTET_STRING:
** CID 435765: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
/grub-core/lib/libtasn1/lib/decoding.c: 439 in asn1_get_object_id_der()
________________________________________________________________________________________________________
*** CID 435765: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
/grub-core/lib/libtasn1/lib/decoding.c: 439 in asn1_get_object_id_der()
433 return ASN1_DER_ERROR;
434
435 val0 = 0;
436
437 for (k = 0; k < len; k++)
438 {
>>> CID 435765: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
>>> "val0 < ((((1 ? 0 : val0) - 1 < 0) ? ~((((1 ? 0 : val0) + 1 << 62UL /*
>>> sizeof (+val0) * 8 - 2 */) - 1) * 2 + 1) : ((1 ? 0 : val0) + 0)) >> 7)" is
>>> always false regardless of the values of its operands. This occurs as the
>>> second operand of "?:".
439 if (INT_LEFT_SHIFT_OVERFLOW (val0, 7))
440 return ASN1_DER_ERROR;
441
442 val0 <<= 7;
443 val0 |= der[len_len + k] & 0x7F;
444 if (!(der[len_len + k] & 0x80))
** CID 435764: Integer handling issues (NO_EFFECT)
/grub-core/lib/libtasn1/lib/decoding.c: 137 in asn1_get_length_der()
________________________________________________________________________________________________________
*** CID 435764: Integer handling issues (NO_EFFECT)
/grub-core/lib/libtasn1/lib/decoding.c: 137 in asn1_get_length_der()
131 punt = 1;
132 if (k)
133 { /* definite length method */
134 ans = 0;
135 while (punt <= k && punt < der_len)
136 {
>>> CID 435764: Integer handling issues (NO_EFFECT)
>>> This less-than-zero comparison of an unsigned value is never true. "(1
>>> ? 0U : ((1 ? 0U : ans) + 256U)) - 1U < 0U".
137 if (INT_MULTIPLY_OVERFLOW (ans, 256))
138 return -2;
139 ans *= 256;
140
141 if (INT_ADD_OVERFLOW (ans, ((unsigned) der[punt])))
142 return -2;
** CID 435763: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
/grub-core/lib/libtasn1/lib/decoding.c: 137 in asn1_get_length_der()
________________________________________________________________________________________________________
*** CID 435763: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
/grub-core/lib/libtasn1/lib/decoding.c: 137 in asn1_get_length_der()
131 punt = 1;
132 if (k)
133 { /* definite length method */
134 ans = 0;
135 while (punt <= k && punt < der_len)
136 {
>>> CID 435763: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
>>> "ans < (((1 ? 0 : ((1 ? 0 : ans) + 256)) - 1 < 0) ? ~((((1 ? 0 : ((1 ?
>>> 0 : ans) + 256)) + 1 << 30UL /* sizeof (+((1 ? 0 : ans) + 256)) * 8 - 2 */)
>>> - 1) * 2 + 1) : ((1 ? 0 : ((1 ? 0 : ans) + 256)) + 0)) / 256" is always
>>> false regardless of the values of its operands. This occurs as the second
>>> operand of "?:".
137 if (INT_MULTIPLY_OVERFLOW (ans, 256))
138 return -2;
139 ans *= 256;
140
141 if (INT_ADD_OVERFLOW (ans, ((unsigned) der[punt])))
142 return -2;
** CID 435762: Memory - corruptions (OVERRUN)
/grub-core/lib/libtasn1/lib/coding.c: 152 in _asn1_tag_der()
________________________________________________________________________________________________________
*** CID 435762: Memory - corruptions (OVERRUN)
/grub-core/lib/libtasn1/lib/coding.c: 152 in _asn1_tag_der()
146 if (k > ASN1_MAX_TAG_SIZE - 1)
147 break; /* will not encode larger tags */
148 }
149 *ans_len = k + 1;
150 while (k--)
151 ans[*ans_len - 1 - k] = temp[k] + 128;
>>> CID 435762: Memory - corruptions (OVERRUN)
>>> Overrunning array of 4 bytes at byte offset 4 by dereferencing pointer
>>> "ans + (*ans_len - 1)".
152 ans[*ans_len - 1] -= 128;
153 }
154 }
155
156 /**
157 * asn1_octet_der:
** CID 435761: Integer handling issues (NEGATIVE_RETURNS)
________________________________________________________________________________________________________
*** CID 435761: Integer handling issues (NEGATIVE_RETURNS)
/util/grub-protect.c: 260 in grub_protect_read_file()
254 err = GRUB_ERR_FILE_READ_ERROR;
255 goto exit1;
256 }
257
258 rewind (f);
259
>>> CID 435761: Integer handling issues (NEGATIVE_RETURNS)
>>> "len" is passed to a parameter that cannot be negative.
260 buf = grub_malloc (len);
261 if (buf == NULL)
262 {
263 err = GRUB_ERR_OUT_OF_MEMORY;
264 goto exit1;
265 }
________________________________________________________________________________________________________
To view the defects in Coverity Scan visit,
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yrkVfgDS2kXmgKKqI9QA-2BVNoKK9q7SFpWGd3m11cukzsw-3D-3DHheQ_pqn7nzZAiyf0hBS-2Bvrr6KCOYHbMcAEkU4V1pyJ3pOgs3pZi4ADKvKcb1upZ6ha0yK6rBruvdhfB2bMQUK0oTfB-2BmmO7FWTlT8Q5-2BK-2FhLtvLedURDxKrS-2F-2Fu4NC-2FvDExIZEGC5875AQV99KngzPjcntrJ9pI7881eNQK6iUKSF8c8EVFpPotSz8Gdch4PkS4ifsHhJXRzb9tKRTpiDXPV3A-3D-3D
To manage Coverity Scan email notifications for "dki...@net-space.pl", click
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXxaL5v6wpPWFsnxwMNLnBswWpckve7onnQrbVzEOkeh9Xj7WV2EPa18sBXMRAENqUVq6r9iyHPR1kC40r-2F2raBCBdK3ZqYVQCFAD4uZPmTSzA-3D-za2_pqn7nzZAiyf0hBS-2Bvrr6KCOYHbMcAEkU4V1pyJ3pOgs3pZi4ADKvKcb1upZ6ha0yemkjYC26UmaxrZoePKKFgIJtj0eah3FtFeSTXcGscF-2FK-2BIKRBDpumaA31JWJVTNFjO4VU9bQuKiYkGvXD3gO7fIzDQbTR7-2BPp8EcxE7go0X9ZqODMbiGnyEH-2B-2Bl58uCmUGTa0JXtHEUpUXG-2FlgqaUg-3D-3D
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
