Skip to content

Commit ea6700f

Browse files
danielinuxdanielinux
committed
Fix sign header TLV overflow sizing
F/2266
1 parent 1353e93 commit ea6700f

2 files changed

Lines changed: 184 additions & 9 deletions

File tree

‎tools/keytools/sign.c‎

Lines changed: 130 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,23 @@ static void header_append_u16(uint8_t* header, uint32_t* idx, uint16_t tmp16)
222222
memcpy(&header[*idx], &tmp16, sizeof(tmp16));
223223
*idx += sizeof(tmp16);
224224
}
225+
226+
static uint32_t header_append_limit(void);
227+
225228
static void header_append_tag(uint8_t* header, uint32_t* idx, uint16_t tag,
226-
uint16_t len, void* data)
229+
uint16_t len, const void* data)
227230
{
231+
const uint32_t append_sz = (uint32_t)(sizeof(tag) + sizeof(len)) + len;
232+
const uint32_t header_sz = header_append_limit();
233+
234+
if ((*idx > header_sz) || (append_sz > (header_sz - *idx))) {
235+
fprintf(stderr,
236+
"Header overflow while appending tag 0x%04x "
237+
"(offset=%u, size=%u, header=%u)\n",
238+
tag, *idx, append_sz, header_sz);
239+
exit(1);
240+
}
241+
228242
header_append_u16(header, idx, tag);
229243
header_append_u16(header, idx, len);
230244
memcpy(&header[*idx], data, len);
@@ -297,6 +311,11 @@ static struct cmd_options CMD = {
297311
.hybrid = 0
298312
};
299313

314+
static uint32_t header_append_limit(void)
315+
{
316+
return CMD.header_sz;
317+
}
318+
300319
static void zero_and_free(uint8_t *buf, uint32_t len)
301320
{
302321
volatile uint8_t *p;
@@ -1125,6 +1144,114 @@ static int sign_digest(int sign, int hash_algo,
11251144
#define ALIGN_8(x) while ((x % 8) != 4) { x++; }
11261145
#define ALIGN_4(x) while ((x % 4) != 0) { x++; }
11271146

1147+
static void header_size_align_8(uint32_t *idx)
1148+
{
1149+
while ((*idx % 8U) != 4U) {
1150+
(*idx)++;
1151+
}
1152+
}
1153+
1154+
static void header_size_align_4(uint32_t *idx)
1155+
{
1156+
while ((*idx % 4U) != 0U) {
1157+
(*idx)++;
1158+
}
1159+
}
1160+
1161+
static void header_size_append_tag(uint32_t *idx, uint32_t len)
1162+
{
1163+
*idx += 4U + len;
1164+
}
1165+
1166+
static uint32_t header_digest_size(int hash_algo)
1167+
{
1168+
switch (hash_algo) {
1169+
case HASH_SHA256:
1170+
return HDR_SHA256_LEN;
1171+
case HASH_SHA384:
1172+
return HDR_SHA384_LEN;
1173+
case HASH_SHA3:
1174+
return HDR_SHA3_384_LEN;
1175+
default:
1176+
return 0;
1177+
}
1178+
}
1179+
1180+
static uint32_t header_required_size(int is_diff, uint32_t cert_chain_sz,
1181+
uint32_t secondary_key_sz)
1182+
{
1183+
uint32_t idx = 0;
1184+
uint32_t digest_sz = header_digest_size(CMD.hash_algo);
1185+
uint32_t i;
1186+
1187+
idx += 2U * sizeof(uint32_t);
1188+
header_size_append_tag(&idx, HDR_VERSION_LEN);
1189+
header_size_align_8(&idx);
1190+
1191+
if (!CMD.no_ts) {
1192+
header_size_append_tag(&idx, HDR_TIMESTAMP_LEN);
1193+
}
1194+
1195+
header_size_append_tag(&idx, HDR_IMG_TYPE_LEN);
1196+
1197+
if (is_diff) {
1198+
header_size_align_4(&idx);
1199+
header_size_append_tag(&idx, 4);
1200+
header_size_append_tag(&idx, 4);
1201+
header_size_align_4(&idx);
1202+
header_size_append_tag(&idx, 4);
1203+
header_size_append_tag(&idx, 4);
1204+
1205+
if (!CMD.no_base_sha && digest_sz > 0U) {
1206+
header_size_align_8(&idx);
1207+
header_size_append_tag(&idx, digest_sz);
1208+
}
1209+
}
1210+
1211+
for (i = 0; i < CMD.custom_tlvs; i++) {
1212+
header_size_align_8(&idx);
1213+
header_size_append_tag(&idx, CMD.custom_tlv[i].len);
1214+
}
1215+
1216+
if (cert_chain_sz > 0U) {
1217+
header_size_align_8(&idx);
1218+
header_size_append_tag(&idx, cert_chain_sz);
1219+
}
1220+
1221+
if (digest_sz > 0U) {
1222+
header_size_align_8(&idx);
1223+
header_size_append_tag(&idx, digest_sz);
1224+
header_size_align_8(&idx);
1225+
1226+
if (CMD.hybrid && secondary_key_sz > 0U) {
1227+
header_size_append_tag(&idx, 2);
1228+
header_size_align_8(&idx);
1229+
header_size_append_tag(&idx, digest_sz);
1230+
header_size_align_8(&idx);
1231+
}
1232+
1233+
header_size_append_tag(&idx, digest_sz);
1234+
}
1235+
1236+
if (CMD.sign != NO_SIGN) {
1237+
header_size_align_8(&idx);
1238+
header_size_append_tag(&idx, CMD.signature_sz);
1239+
1240+
if (CMD.hybrid) {
1241+
header_size_align_8(&idx);
1242+
header_size_append_tag(&idx, CMD.secondary_signature_sz);
1243+
}
1244+
1245+
if (CMD.policy_sign) {
1246+
header_size_align_8(&idx);
1247+
header_size_append_tag(&idx,
1248+
CMD.policy_sz + (uint32_t)sizeof(uint32_t));
1249+
}
1250+
}
1251+
1252+
return idx;
1253+
}
1254+
11281255
static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz,
11291256
const char *image_file, const char *outfile,
11301257
uint32_t delta_base_version, uint32_t patch_len, uint32_t patch_inv_off,
@@ -1158,14 +1285,8 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz,
11581285

11591286
/* Get the file size */
11601287
if (stat(CMD.cert_chain_file, &file_stat) == 0) {
1161-
/* 2 bytes for tag + 2 bytes for length field */
1162-
const uint32_t tag_len_size = 4;
1163-
/* Maximum alignment padding that might be needed */
1164-
const uint32_t max_alignment = 8;
1165-
/* Required space = tag(2) + length(2) + data + potential alignment
1166-
* * padding */
1167-
const uint32_t required_space =
1168-
tag_len_size + file_stat.st_size + max_alignment;
1288+
const uint32_t required_space = header_required_size(is_diff,
1289+
(uint32_t)file_stat.st_size, secondary_key_sz);
11691290

11701291
/* If the current header size is too small, increase it */
11711292
if (CMD.header_sz < required_space) {

‎tools/unit-tests/unit-sign-encrypted-output.c‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,13 +220,67 @@ START_TEST(test_make_header_ex_fails_when_image_reopen_fails)
220220
}
221221
END_TEST
222222

223+
START_TEST(test_make_header_ex_grows_header_for_cert_chain_and_digest_tlvs)
224+
{
225+
char tempdir[] = "/tmp/wolfboot-sign-XXXXXX";
226+
char image_path[PATH_MAX];
227+
char output_path[PATH_MAX];
228+
char cert_chain_path[PATH_MAX];
229+
uint8_t image_buf[] = { 0x01, 0x02, 0x03, 0x04 };
230+
uint8_t cert_chain_buf[200];
231+
uint8_t pubkey[] = { 0xA5 };
232+
struct stat st;
233+
int ret;
234+
235+
ck_assert_ptr_nonnull(mkdtemp(tempdir));
236+
237+
snprintf(image_path, sizeof(image_path), "%s/image.bin", tempdir);
238+
snprintf(output_path, sizeof(output_path), "%s/output.bin", tempdir);
239+
snprintf(cert_chain_path, sizeof(cert_chain_path), "%s/cert-chain.bin",
240+
tempdir);
241+
242+
memset(cert_chain_buf, 0xC3, sizeof(cert_chain_buf));
243+
ck_assert_int_eq(write_file(image_path, image_buf, sizeof(image_buf)), 0);
244+
ck_assert_int_eq(write_file(cert_chain_path, cert_chain_buf,
245+
sizeof(cert_chain_buf)), 0);
246+
247+
memset(&CMD, 0, sizeof(CMD));
248+
CMD.sign = NO_SIGN;
249+
CMD.hash_algo = HASH_SHA256;
250+
CMD.partition_id = HDR_IMG_TYPE_APP;
251+
CMD.header_sz = 256;
252+
CMD.fw_version = "7";
253+
CMD.no_ts = 1;
254+
CMD.cert_chain_file = cert_chain_path;
255+
256+
reset_mocks(NULL, 0);
257+
ret = make_header_ex(0, pubkey, sizeof(pubkey), image_path, output_path,
258+
0, 0, 0, 0, NULL, 0, NULL, 0);
259+
260+
ck_assert_int_eq(ret, 0);
261+
ck_assert_uint_eq(CMD.header_sz, 512);
262+
ck_assert_int_eq(stat(output_path, &st), 0);
263+
ck_assert_uint_eq((uint32_t)st.st_size, CMD.header_sz + sizeof(image_buf));
264+
ck_assert_int_eq(mock_null_fwrite_calls, 0);
265+
ck_assert_int_eq(mock_null_fread_calls, 0);
266+
ck_assert_int_eq(mock_null_fclose_calls, 0);
267+
268+
unlink(output_path);
269+
unlink(cert_chain_path);
270+
unlink(image_path);
271+
rmdir(tempdir);
272+
}
273+
END_TEST
274+
223275
Suite *wolfboot_suite(void)
224276
{
225277
Suite *s = suite_create("sign-encrypted-output");
226278
TCase *tcase = tcase_create("make-header-ex");
227279

228280
tcase_add_test(tcase, test_make_header_ex_fails_when_encrypted_output_open_fails);
229281
tcase_add_test(tcase, test_make_header_ex_fails_when_image_reopen_fails);
282+
tcase_add_test(tcase,
283+
test_make_header_ex_grows_header_for_cert_chain_and_digest_tlvs);
230284
suite_add_tcase(s, tcase);
231285

232286
return s;

0 commit comments

Comments
 (0)