fix(p6-batch): reject clean EOF on record read; add traversal/EOF security tests
This commit is contained in:
@@ -46,6 +46,7 @@ void print_usage(void) {
|
||||
printf(" wire formats)\n");
|
||||
printf(" --write-batch=FILE Run the normal live transfer AND also emit a\n");
|
||||
printf(" self-contained batch file of the whole source tree\n");
|
||||
printf(" (implies the single-threaded transfer path)\n");
|
||||
printf(" --only-write-batch=FILE\n");
|
||||
printf(" Emit the batch file only (no destination, no server)\n");
|
||||
printf(" --read-batch=FILE Apply the batch file to the destination (no source, no\n");
|
||||
|
||||
+1
-1
@@ -128,7 +128,7 @@ int batch_read_apply(int fd, const Config* config, const char* dest_root) {
|
||||
log_message(LOG_LEVEL_ERROR, "batch: could not allocate a %llu-byte record", length);
|
||||
return -1;
|
||||
}
|
||||
if (!read_exact(fd, record, (size_t)length, &eof)) {
|
||||
if (!read_exact(fd, record, (size_t)length, &eof) || eof) {
|
||||
log_message(LOG_LEVEL_ERROR, "batch: truncated chunk record");
|
||||
free(record);
|
||||
return -1;
|
||||
|
||||
+6
-2
@@ -13,8 +13,12 @@
|
||||
#define BATCH_MAGIC "FSTRESBATCH"
|
||||
#define BATCH_MAGIC_LEN 11
|
||||
#define BATCH_FORMAT_VERSION 1
|
||||
/* A single deserialized record is bounded by the chunk codec's 64 MB cap
|
||||
* (the same per-file data cap chunk_deserialize enforces). */
|
||||
/* Max size of a single length-prefixed record (a whole serialized chunk,
|
||||
* which can span several files). A single source file near the 64 MB wire
|
||||
* limit plus per-file headers can produce a record slightly over 64 MB, so a
|
||||
* large file just under the wire cap may be refused by the batch writer; this
|
||||
* is documented upstream and the failure is clean (the partial batch is
|
||||
* unlinked), never a truncated/corrupt batch. */
|
||||
#define BATCH_MAX_RECORD (64ULL * 1024 * 1024)
|
||||
|
||||
bool batch_write_header(int fd, const Config* config);
|
||||
|
||||
@@ -179,10 +179,77 @@ static void test_batch_reject_oversized() {
|
||||
unlink("batch_big.bin");
|
||||
}
|
||||
|
||||
/* A clean header followed by a length prefix with NO record bytes at all (clean
|
||||
* EOF on the record-body read) must be rejected as truncated — it must not feed
|
||||
* an uninitialized buffer to chunk_deserialize. Regression test for a
|
||||
* confirmed uninitialized-read on the untrusted read side. */
|
||||
static void test_batch_reject_eof_after_prefix() {
|
||||
Config* config = config_create();
|
||||
EXPECT_NOT_NULL(config);
|
||||
int fd = open("batch_eof.bin", O_WRONLY | O_CREAT | O_TRUNC, 0644);
|
||||
EXPECT_TRUE(fd >= 0);
|
||||
EXPECT_TRUE(batch_write_header(fd, config));
|
||||
unsigned long long length = 32;
|
||||
EXPECT_EQ_INT(write(fd, &length, sizeof(length)), (ssize_t)sizeof(length));
|
||||
EXPECT_EQ_INT(close(fd), 0);
|
||||
fd = open("batch_eof.bin", O_RDONLY);
|
||||
EXPECT_TRUE(fd >= 0);
|
||||
EXPECT_EQ_INT(batch_read_apply(fd, config, "batch_dest"), -1);
|
||||
EXPECT_EQ_INT(close(fd), 0);
|
||||
config_delete(config);
|
||||
unlink("batch_eof.bin");
|
||||
}
|
||||
|
||||
/* A malicious batch record whose chunk carries a path-traversal wire path must
|
||||
* be refused by the apply path — never applied outside the destination root.
|
||||
* We craft a chunk whose wire path is `../escape.txt` (the local source file
|
||||
* is a benign temp file; only the transmitted path is hostile) and assert the
|
||||
* apply refuses it and nothing is created outside the root. */
|
||||
static void test_batch_reject_traversal_path() {
|
||||
const char* content = "hostile traversal image\n";
|
||||
size_t content_len = strlen(content);
|
||||
file_write_to_disk("batch_trav_src.txt", content, content_len, false, false);
|
||||
|
||||
struct stat st;
|
||||
EXPECT_EQ_INT(stat("batch_trav_src.txt", &st), 0);
|
||||
File* f = file_create("batch_trav_src.txt");
|
||||
EXPECT_NOT_NULL(f);
|
||||
f->data->size = (unsigned long long)st.st_size;
|
||||
EXPECT_TRUE(file_load_data(f));
|
||||
f->send_path = str_dup("../escape.txt");
|
||||
EXPECT_NOT_NULL(f->send_path);
|
||||
File* files[1] = {f};
|
||||
Chunk* chunk = chunk_create(files, 1);
|
||||
EXPECT_NOT_NULL(chunk);
|
||||
|
||||
Config* config = config_create();
|
||||
EXPECT_NOT_NULL(config);
|
||||
|
||||
int wfd = open("batch_trav.bin", O_WRONLY | O_CREAT | O_TRUNC, 0644);
|
||||
EXPECT_TRUE(wfd >= 0);
|
||||
EXPECT_TRUE(batch_write_header(wfd, config));
|
||||
EXPECT_TRUE(batch_write_chunk(wfd, chunk));
|
||||
EXPECT_EQ_INT(close(wfd), 0);
|
||||
chunk_destroy(chunk); /* frees f and f->send_path */
|
||||
|
||||
int rfd = open("batch_trav.bin", O_RDONLY);
|
||||
EXPECT_TRUE(rfd >= 0);
|
||||
EXPECT_EQ_INT(batch_read_apply(rfd, config, "batch_dest"), -1);
|
||||
EXPECT_EQ_INT(close(rfd), 0);
|
||||
unlink("../escape.txt"); /* clear any stale file so the probe below is clean */
|
||||
EXPECT_TRUE(access("../escape.txt", F_OK) != 0);
|
||||
|
||||
config_delete(config);
|
||||
unlink("batch_trav.bin");
|
||||
unlink("batch_trav_src.txt");
|
||||
}
|
||||
|
||||
void test_batch() {
|
||||
test_batch_roundtrip();
|
||||
test_batch_roundtrip_metadata();
|
||||
test_batch_reject_bad_magic();
|
||||
test_batch_reject_truncated();
|
||||
test_batch_reject_oversized();
|
||||
test_batch_reject_eof_after_prefix();
|
||||
test_batch_reject_traversal_path();
|
||||
}
|
||||
Reference in New Issue
Block a user