Skip to content

Commit 219c1ba

Browse files
petrelharppetrelharp
committed
memory alignment in tsk_json_struct_metadata_get_blob
1 parent 9eb9455 commit 219c1ba

File tree

3 files changed

+81
-20
lines changed

3 files changed

+81
-20
lines changed

‎c/tests/test_core.c‎

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ test_json_struct_metadata_get_blob(void)
109109
tsk_size_t metadata_length;
110110
size_t header_length;
111111
size_t json_length;
112+
size_t padding_length;
112113
size_t payload_length;
113114
size_t total_length;
114115
char json_payload[] = "{\"a\":1}";
@@ -118,8 +119,9 @@ test_json_struct_metadata_get_blob(void)
118119
bytes = (uint8_t *) metadata;
119120
header_length = 4 + 1 + 8 + 8;
120121
json_length = strlen(json_payload);
122+
padding_length = (8 - ((header_length + json_length) & 0x07)) % 8;
121123
payload_length = sizeof(binary_payload);
122-
total_length = header_length + json_length + payload_length;
124+
total_length = header_length + json_length + padding_length + payload_length;
123125
CU_ASSERT_FATAL(total_length <= sizeof(metadata));
124126
memset(metadata, 0, sizeof(metadata));
125127
bytes[0] = 'J';
@@ -130,54 +132,64 @@ test_json_struct_metadata_get_blob(void)
130132
set_u64_le(bytes + 5, (uint64_t) json_length);
131133
set_u64_le(bytes + 13, (uint64_t) payload_length);
132134
memcpy(bytes + header_length, json_payload, json_length);
133-
memcpy(bytes + header_length + json_length, binary_payload, payload_length);
135+
memset(bytes + header_length + json_length, 0, padding_length);
136+
memcpy(bytes + header_length + json_length + padding_length, binary_payload,
137+
payload_length);
134138
metadata_length = (tsk_size_t) total_length;
135139
ret = tsk_json_struct_metadata_get_blob(
136140
metadata, metadata_length, &json, &json_buffer_length, &blob, &blob_length);
137141
CU_ASSERT_EQUAL(ret, 0);
138142
CU_ASSERT_PTR_EQUAL(json, (char *) bytes + header_length);
143+
CU_ASSERT_EQUAL(json + json_buffer_length + padding_length, blob);
139144
CU_ASSERT_EQUAL(json_buffer_length, (tsk_size_t) json_length);
140145
if (json_length > 0) {
141146
CU_ASSERT_EQUAL(memcmp(json, json_payload, json_length), 0);
142147
}
143-
CU_ASSERT_PTR_EQUAL(blob, bytes + header_length + json_length);
148+
CU_ASSERT_PTR_EQUAL(blob, bytes + header_length + json_length + padding_length);
144149
CU_ASSERT_EQUAL(blob_length, (tsk_size_t) payload_length);
145150
CU_ASSERT_EQUAL(memcmp(blob, binary_payload, payload_length), 0);
151+
CU_ASSERT((tsk_size_t) (blob - json) < json_buffer_length + 8);
146152

147153
payload_length = 0;
148-
total_length = header_length + json_length + payload_length;
154+
total_length = header_length + json_length + padding_length + payload_length;
149155
CU_ASSERT_FATAL(total_length <= sizeof(metadata));
150156
set_u64_le(bytes + 13, (uint64_t) payload_length);
151157
metadata_length = (tsk_size_t) total_length;
152158
ret = tsk_json_struct_metadata_get_blob(
153159
metadata, metadata_length, &json, &json_buffer_length, &blob, &blob_length);
154160
CU_ASSERT_EQUAL(ret, 0);
161+
CU_ASSERT_EQUAL(json + json_buffer_length + padding_length, blob);
155162
CU_ASSERT_PTR_EQUAL(json, (char *) bytes + header_length);
156163
CU_ASSERT_EQUAL(json_buffer_length, (tsk_size_t) json_length);
157164
CU_ASSERT_EQUAL(blob_length, (tsk_size_t) payload_length);
158-
CU_ASSERT_PTR_EQUAL(blob, bytes + header_length + json_length);
165+
CU_ASSERT_PTR_EQUAL(blob, bytes + header_length + json_length + padding_length);
166+
CU_ASSERT((tsk_size_t) (blob - json) < json_buffer_length + 8);
159167

160168
json_length = 0;
161169
payload_length = sizeof(empty_payload);
162-
total_length = header_length + json_length + payload_length;
170+
padding_length = (8 - ((header_length + json_length) & 0x07)) % 8;
171+
total_length = header_length + json_length + padding_length + payload_length;
163172
CU_ASSERT_FATAL(total_length <= sizeof(metadata));
164173
set_u64_le(bytes + 5, (uint64_t) json_length);
165174
set_u64_le(bytes + 13, (uint64_t) payload_length);
166-
memcpy(bytes + header_length + json_length, empty_payload, payload_length);
175+
memset(bytes + header_length + json_length, 0, padding_length);
176+
memcpy(bytes + header_length + json_length + padding_length, empty_payload,
177+
payload_length);
167178
metadata_length = (tsk_size_t) total_length;
168179
ret = tsk_json_struct_metadata_get_blob(
169180
metadata, metadata_length, &json, &json_buffer_length, &blob, &blob_length);
170181
CU_ASSERT_EQUAL(ret, 0);
171182
CU_ASSERT_PTR_EQUAL(json, (char *) bytes + header_length);
172183
CU_ASSERT_EQUAL(json_buffer_length, (tsk_size_t) json_length);
173184
CU_ASSERT_EQUAL(blob_length, (tsk_size_t) payload_length);
174-
CU_ASSERT_PTR_EQUAL(blob, bytes + header_length + json_length);
185+
CU_ASSERT_PTR_EQUAL(blob, bytes + header_length + json_length + padding_length);
175186
CU_ASSERT_EQUAL(memcmp(blob, empty_payload, payload_length), 0);
187+
CU_ASSERT((tsk_size_t) (blob - json) < json_buffer_length + 8);
176188

177189
blob = NULL;
178190
blob_length = 0;
179191
json = NULL;
180-
json_buffer_length = 0;
192+
json_length = 0;
181193
metadata_length = header_length - 1;
182194
ret = tsk_json_struct_metadata_get_blob(
183195
metadata, metadata_length, &json, &json_buffer_length, &blob, &blob_length);
@@ -196,7 +208,19 @@ test_json_struct_metadata_get_blob(void)
196208
CU_ASSERT_EQUAL(ret, TSK_ERR_JSON_STRUCT_METADATA_BAD_VERSION);
197209
bytes[4] = 1;
198210

199-
metadata_length = (tsk_size_t) (total_length - 1);
211+
set_u64_le(bytes + 5, (uint64_t) json_length + 9);
212+
ret = tsk_json_struct_metadata_get_blob(
213+
metadata, metadata_length, &json, &json_buffer_length, &blob, &blob_length);
214+
CU_ASSERT_EQUAL(ret, TSK_ERR_JSON_STRUCT_METADATA_UNEXPECTED_SIZE);
215+
set_u64_le(bytes + 5, (uint64_t) json_length);
216+
217+
bytes[header_length + 1] = 1;
218+
ret = tsk_json_struct_metadata_get_blob(
219+
metadata, metadata_length, &json, &json_buffer_length, &blob, &blob_length);
220+
CU_ASSERT_EQUAL(ret, TSK_ERR_JSON_STRUCT_METADATA_NONZERO_PADDING);
221+
bytes[header_length + 1] = 0;
222+
223+
metadata_length = (tsk_size_t) (header_length - 1);
200224
ret = tsk_json_struct_metadata_get_blob(
201225
metadata, metadata_length, &json, &json_buffer_length, &blob, &blob_length);
202226
CU_ASSERT_EQUAL(ret, TSK_ERR_JSON_STRUCT_METADATA_TRUNCATED);

‎c/tskit/core.c‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,13 @@ tsk_json_struct_metadata_get_blob(char *metadata, tsk_size_t metadata_length,
150150
uint64_t json_length_u64;
151151
uint64_t binary_length_u64;
152152
uint64_t header_and_json_length;
153+
uint64_t padding_length;
154+
uint64_t header_and_json_and_padding_length;
153155
uint64_t total_length;
154156
uint8_t *bytes;
155-
char *blob_start;
156157
char *json_start;
158+
char *padding_start;
159+
char *blob_start;
157160

158161
if (metadata == NULL || json == NULL || json_length == NULL || blob == NULL
159162
|| blob_length == NULL) {
@@ -181,17 +184,32 @@ tsk_json_struct_metadata_get_blob(char *metadata, tsk_size_t metadata_length,
181184
goto out;
182185
}
183186
header_and_json_length = (uint64_t) TSK_JSON_BINARY_HEADER_SIZE + json_length_u64;
184-
if (binary_length_u64 > UINT64_MAX - header_and_json_length) {
187+
padding_length = (8 - (header_and_json_length & 0x07)) % 8;
188+
if (padding_length > UINT64_MAX - header_and_json_length) {
185189
ret = tsk_trace_error(TSK_ERR_JSON_STRUCT_METADATA_INVALID_LENGTH);
186190
goto out;
187191
}
188-
total_length = header_and_json_length + binary_length_u64;
189-
if ((uint64_t) metadata_length < total_length) {
190-
ret = tsk_trace_error(TSK_ERR_JSON_STRUCT_METADATA_TRUNCATED);
192+
header_and_json_and_padding_length = header_and_json_length + padding_length;
193+
if (binary_length_u64 > UINT64_MAX - header_and_json_and_padding_length) {
194+
ret = tsk_trace_error(TSK_ERR_JSON_STRUCT_METADATA_INVALID_LENGTH);
195+
goto out;
196+
}
197+
total_length = header_and_json_and_padding_length + binary_length_u64;
198+
if ((uint64_t) metadata_length != total_length) {
199+
ret = tsk_trace_error(TSK_ERR_JSON_STRUCT_METADATA_UNEXPECTED_SIZE);
191200
goto out;
192201
}
202+
padding_start = (char *) bytes + TSK_JSON_BINARY_HEADER_SIZE + json_length_u64;
203+
for (uint64_t padding_index = 0; padding_index < padding_length; ++padding_index) {
204+
// require padding bytes to be zero, for a bit more safety
205+
if (*(padding_start + padding_index) != (char) 0) {
206+
ret = tsk_trace_error(TSK_ERR_JSON_STRUCT_METADATA_NONZERO_PADDING);
207+
goto out;
208+
}
209+
}
193210
json_start = (char *) bytes + TSK_JSON_BINARY_HEADER_SIZE;
194-
blob_start = (char *) bytes + TSK_JSON_BINARY_HEADER_SIZE + json_length_u64;
211+
blob_start = (char *) bytes + TSK_JSON_BINARY_HEADER_SIZE + json_length_u64
212+
+ padding_length;
195213
*json = json_start;
196214
*json_length = (tsk_size_t) json_length_u64;
197215
*blob = blob_start;
@@ -283,6 +301,14 @@ tsk_strerror_internal(int err)
283301
ret = "JSON binary struct metadata uses an unsupported version number. "
284302
"(TSK_ERR_JSON_STRUCT_METADATA_BAD_VERSION)";
285303
break;
304+
case TSK_ERR_JSON_STRUCT_METADATA_UNEXPECTED_SIZE:
305+
ret = "JSON binary struct metadata is not equal to the expected size. "
306+
"(TSK_ERR_JSON_STRUCT_METADATA_UNEXPECTED_SIZE)";
307+
break;
308+
case TSK_ERR_JSON_STRUCT_METADATA_NONZERO_PADDING:
309+
ret = "JSON binary struct metadata has non-zero padding bytes between the "
310+
"JSON and struct. (TSK_ERR_JSON_STRUCT_METADATA_NONZERO_PADDING)";
311+
break;
286312

287313
/* Out of bounds errors */
288314
case TSK_ERR_BAD_OFFSET:

‎c/tskit/core.h‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,16 @@ A length field in the JSON binary struct metadata header is invalid.
329329
The JSON binary struct metadata uses an unsupported version number.
330330
*/
331331
#define TSK_ERR_JSON_STRUCT_METADATA_BAD_VERSION -109
332+
333+
/**
334+
The JSON binary struct metadata is not equal to the expected size.
335+
*/
336+
#define TSK_ERR_JSON_STRUCT_METADATA_UNEXPECTED_SIZE -110
337+
338+
/**
339+
The JSON binary struct metadata padding bytes are not zeroed.
340+
*/
341+
#define TSK_ERR_JSON_STRUCT_METADATA_NONZERO_PADDING -111
332342
/** @} */
333343

334344
/**
@@ -1136,10 +1146,11 @@ int tsk_generate_uuid(char *dest, int flags);
11361146
@brief Extract the binary payload from ``json+struct`` encoded metadata.
11371147
11381148
@rst
1139-
Metadata produced by the JSONStructCodec consists of a fixed-size
1140-
header followed by canonical JSON bytes and an optional binary payload. This helper
1141-
validates the framing, returning pointers to the embedded JSON and binary sections
1142-
without copying.
1149+
Metadata produced by the JSONStructCodec consists of a fixed-size header followed
1150+
by canonical JSON bytes, a variable number of padding bytes (which should be zero)
1151+
to bring the length to a multiple of 8 bytes for alignment, and finally an optional
1152+
binary payload. This helper validates the framing, returning pointers to the
1153+
embedded JSON and binary sections without copying.
11431154
11441155
The output pointers reference memory owned by the caller and remain valid only while
11451156
the original metadata buffer is alive.

0 commit comments

Comments
 (0)