Fix a toctou bug in libzip by removing the entire unused function

This commit is contained in:
pancake 2024-05-16 19:37:00 +02:00 committed by pancake
parent 85238782f5
commit 6bb987c28d
2 changed files with 8 additions and 204 deletions

View File

@ -65,7 +65,7 @@ zip_source_file_common_new(const char *fname, void *file, zip_uint64_t start, zi
return NULL;
}
if (ops->write != NULL && (ops->commit_write == NULL || ops->create_temp_output == NULL || ops->remove == NULL || ops->rollback_write == NULL || ops->tell == NULL)) {
if (ops->write != NULL && (ops->commit_write == NULL || ops->remove == NULL || ops->rollback_write == NULL || ops->tell == NULL)) {
zip_error_set(error, ZIP_ER_INTERNAL, 0);
return NULL;
}
@ -218,6 +218,9 @@ read_file(void *state, void *data, zip_uint64_t len, zip_source_cmd_t cmd) {
zip_error_set(&ctx->error, ZIP_ER_INTERNAL, 0);
return -1;
}
if (ctx->ops->create_temp_output == NULL) {
return -1;
}
return ctx->ops->create_temp_output(ctx);
case ZIP_SOURCE_BEGIN_WRITE_CLONING:
@ -226,6 +229,9 @@ read_file(void *state, void *data, zip_uint64_t len, zip_source_cmd_t cmd) {
zip_error_set(&ctx->error, ZIP_ER_INTERNAL, 0);
return -1;
}
if (ctx->ops->create_temp_output_cloning == NULL) {
return -1;
}
return ctx->ops->create_temp_output_cloning(ctx, len);
case ZIP_SOURCE_CLOSE:

View File

@ -54,10 +54,7 @@
#define CAN_CLONE
#endif
static int create_temp_file(zip_source_file_context_t *ctx, bool create_file);
static zip_int64_t _zip_stdio_op_commit_write(zip_source_file_context_t *ctx);
static zip_int64_t _zip_stdio_op_create_temp_output(zip_source_file_context_t *ctx);
#ifdef CAN_CLONE
static zip_int64_t _zip_stdio_op_create_temp_output_cloning(zip_source_file_context_t *ctx, zip_uint64_t offset);
#endif
@ -71,7 +68,7 @@ static zip_int64_t _zip_stdio_op_write(zip_source_file_context_t *ctx, const voi
static zip_source_file_operations_t ops_stdio_named = {
_zip_stdio_op_close,
_zip_stdio_op_commit_write,
_zip_stdio_op_create_temp_output,
NULL, // _zip_stdio_op_create_temp_output,
#ifdef CAN_CLONE
_zip_stdio_op_create_temp_output_cloning,
#else
@ -124,119 +121,6 @@ _zip_stdio_op_commit_write(zip_source_file_context_t *ctx) {
}
static zip_int64_t
_zip_stdio_op_create_temp_output(zip_source_file_context_t *ctx) {
int fd = create_temp_file(ctx, true);
if (fd < 0) {
return -1;
}
if ((ctx->fout = fdopen(fd, "r+b")) == NULL) {
zip_error_set(&ctx->error, ZIP_ER_TMPOPEN, errno);
close(fd);
(void)remove(ctx->tmpname);
free(ctx->tmpname);
ctx->tmpname = NULL;
return -1;
}
return 0;
}
#ifdef CAN_CLONE
static zip_int64_t
_zip_stdio_op_create_temp_output_cloning(zip_source_file_context_t *ctx, zip_uint64_t offset) {
FILE *tfp;
if (offset > ZIP_OFF_MAX) {
zip_error_set(&ctx->error, ZIP_ER_SEEK, E2BIG);
return -1;
}
#ifdef HAVE_CLONEFILE
/* clonefile insists on creating the file, so just create a name */
if (create_temp_file(ctx, false) < 0) {
return -1;
}
if (clonefile(ctx->fname, ctx->tmpname, 0) < 0) {
zip_error_set(&ctx->error, ZIP_ER_TMPOPEN, errno);
free(ctx->tmpname);
ctx->tmpname = NULL;
return -1;
}
if ((tfp = _zip_fopen_close_on_exec(ctx->tmpname, true)) == NULL) {
zip_error_set(&ctx->error, ZIP_ER_TMPOPEN, errno);
(void)remove(ctx->tmpname);
free(ctx->tmpname);
ctx->tmpname = NULL;
return -1;
}
#else
{
int fd;
struct file_clone_range range;
struct stat st;
if (fstat(fileno(ctx->f), &st) < 0) {
zip_error_set(&ctx->error, ZIP_ER_TMPOPEN, errno);
return -1;
}
if ((fd = create_temp_file(ctx, true)) < 0) {
return -1;
}
range.src_fd = fileno(ctx->f);
range.src_offset = 0;
range.src_length = ((offset + st.st_blksize - 1) / st.st_blksize) * st.st_blksize;
if (range.src_length > st.st_size) {
range.src_length = 0;
}
range.dest_offset = 0;
if (ioctl(fd, FICLONERANGE, &range) < 0) {
zip_error_set(&ctx->error, ZIP_ER_TMPOPEN, errno);
(void)close(fd);
(void)remove(ctx->tmpname);
free(ctx->tmpname);
ctx->tmpname = NULL;
return -1;
}
if ((tfp = fdopen(fd, "r+b")) == NULL) {
zip_error_set(&ctx->error, ZIP_ER_TMPOPEN, errno);
(void)close(fd);
(void)remove(ctx->tmpname);
free(ctx->tmpname);
ctx->tmpname = NULL;
return -1;
}
}
#endif
if (ftruncate(fileno(tfp), (off_t)offset) < 0) {
(void)fclose(tfp);
(void)remove(ctx->tmpname);
free(ctx->tmpname);
ctx->tmpname = NULL;
return -1;
}
if (fseeko(tfp, (off_t)offset, SEEK_SET) < 0) {
zip_error_set(&ctx->error, ZIP_ER_TMPOPEN, errno);
(void)fclose(tfp);
(void)remove(ctx->tmpname);
free(ctx->tmpname);
ctx->tmpname = NULL;
return -1;
}
ctx->fout = tfp;
return 0;
}
#endif
static bool
_zip_stdio_op_open(zip_source_file_context_t *ctx) {
if ((ctx->f = _zip_fopen_close_on_exec(ctx->fname, false)) == NULL) {
@ -284,89 +168,3 @@ _zip_stdio_op_write(zip_source_file_context_t *ctx, const void *data, zip_uint64
return (zip_int64_t)ret;
}
static int create_temp_file(zip_source_file_context_t *ctx, bool create_file) {
char *temp;
int mode;
struct stat st;
int fd;
char *start, *end;
if (stat(ctx->fname, &st) == 0) {
mode = st.st_mode;
}
else {
mode = -1;
}
if ((temp = (char *)malloc(strlen(ctx->fname) + 13)) == NULL) {
zip_error_set(&ctx->error, ZIP_ER_MEMORY, 0);
return -1;
}
sprintf(temp, "%s.XXXXXX.part", ctx->fname);
end = temp + strlen(temp) - 5;
start = end - 6;
for (;;) {
zip_uint32_t value = zip_random_uint32();
char *xs = start;
while (xs < end) {
char digit = value % 36;
if (digit < 10) {
*(xs++) = digit + '0';
}
else {
*(xs++) = digit - 10 + 'a';
}
value /= 36;
}
if (create_file) {
#if _WIN32
if ((fd = open(temp, O_CREAT | O_BINARY | O_RDWR, mode == -1 ? 0666 : mode)) >= 0) {
break;
}
free(temp);
#else
if ((fd = open(temp, O_CREAT | O_EXCL | O_RDWR | O_CLOEXEC, mode == -1 ? 0666 : (mode_t)mode)) >= 0) {
if (mode != -1) {
#if __wasi__
// no chmod here
#else
/* open() honors umask(), which we don't want in this case */
#ifdef HAVE_FCHMOD
(void)fchmod(fd, (mode_t)mode);
#else
(void)chmod(temp, (mode_t)mode);
#endif
#endif
}
break;
}
if (errno != EEXIST) {
zip_error_set(&ctx->error, ZIP_ER_TMPOPEN, errno);
free(temp);
return -1;
}
#endif
}
else {
if (stat(temp, &st) < 0) {
if (errno == ENOENT) {
break;
}
else {
zip_error_set(&ctx->error, ZIP_ER_TMPOPEN, errno);
free(temp);
return -1;
}
}
}
}
ctx->tmpname = temp;
return create_file ? fd : 0;
}