Hey,
On Thu, Aug 02, 2018 at 11:43:58AM +0200, Jakub Jelen wrote:
> diff --git a/tests/hwtests.c b/tests/hwtests.c
> index 7beebac..bd8e439 100644
> --- a/tests/hwtests.c
> +++ b/tests/hwtests.c
> @@ -268,6 +268,85 @@ static void test_get_response_hw(void) {
> test_get_response();
> }
>
> +/* Try to pass bad formatted PKCS#1.5 data and make sure the libcacard does not
> + * crash while handling them
> + */
> +static void test_sign_bad_data_x509(void)
> +{
> + VReader *reader = vreader_get_reader_by_id(0);
> + VReaderStatus status;
> + int dwRecvLength = APDUBufSize;
> + uint8_t pbRecvBuffer[APDUBufSize];
> + uint8_t sign[] = {
> + /* VERIFY [p1,p2=0 ] [Lc ] [2048b keys: 256 bytes of non PKCS#1.5 data] */
> + 0x80, 0x42, 0x00, 0x00, 0x00, 0x01, 0x00,
> +0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +/* ^--- the second byte of data should be 0x01 for signatures */
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +0xff, 0xff, 0x00, 0x64, 0x61, 0x74, 0x61, 0x20, 0x74, 0x6f, 0x20, 0x73, 0x69, 0x67, 0x6e, 0x20,
> +0x28, 0x6d, 0x61, 0x78, 0x20, 0x31, 0x30, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x29, 0x0a,
> +0x00 /* <-- [Le] */
> + };
> + int sign_len = sizeof(sign);
> + int key_bits = getBits();
> +
> + g_assert_nonnull(reader);
> +
> + /* Skip the HW tests without physical card */
> + if (vreader_card_is_present(reader) != VREADER_OK) {
> + vreader_free(reader);
> + g_test_skip("No physical card found");
> + return;
> + }
> +
> + /* run the actual test */
> +
> + key_bits = getBits();
> + /* Adjust the buffers to match the key lengths, if already retrieved */
> + if (key_bits && key_bits < 2048) {
> + sign[4] = key_bits/8; /* less than 2048b will fit the length into one byte */
> + sign[5] = 0x00;
> + /*sign[6] = 0x01; <- this should be 0x01 for PKCS#1.5 signatures */
> + memcpy(&sign[6], &sign[sign_len-key_bits/8 + 3], key_bits/8 - 1);
This memcpy is overflowing 'sign' and is causing make check with
-fsanitize=address to fail. We try to copy key_bits/8 - 1 bytes starting
at offset sign_len-key_bits/8 + 3, which means we'll end up at offset
key_bits/8 - 1 + sign_len - key_bits/8 + 3, which computes to sign_len +
2, while the 'sign' array only has a size of 'sign_len'.
I'm replying to this email as I don't know what the right fix for this
is.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel