aiden
/
suricata
mirror of https://github.com/OISF/suricata
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

output/filestore: improve path handling

Browse Source 
Use PathMerge where appropriate. Addresses format truncation warnings.

output-filestore.c: In function 'OutputFilestoreFinalizeFiles':
output-filestore.c:164:13: warning: '%lu' directive output may be truncated writing between 1 and 14 bytes into a region of size between 0 and 4095 [-Wformat-truncation=]
  164 |         if (snprintf(js_metadata_filename, sizeof(js_metadata_filename), "%s.%" PRIuMAX ".%u.json",
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  165 |                     final_filename, (uintmax_t)SCTIME_SECS(p->ts),
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  166 |                     ff->file_store_id) == (int)sizeof(js_metadata_filename)) {
      |                     ~~~~~~~~~~~~~~~~~~
output-filestore.c:164:13: note: directive argument in the range [0, 17592186044415]
output-filestore.c:164:13: note: 'snprintf' output between 10 and 4127 bytes into a destination of size 4096
output-filestore.c: In function 'OutputFilestoreLogInitCtx':
output-filestore.c:408:67: warning: '%s' directive output may be truncated writing up to 4095 bytes into a region of size 4027 [-Wformat-truncation=]
  408 |     int written = snprintf(ctx->tmpdir, sizeof(ctx->tmpdir) - 1, "%s/tmp",
      |                                                                   ^~
  409 |             log_directory);
      |             ~~~~~~~~~~~~~
output-filestore.c:408:19: note: 'snprintf' output between 5 and 4100 bytes into a destination of size 4027
  408 |     int written = snprintf(ctx->tmpdir, sizeof(ctx->tmpdir) - 1, "%s/tmp",
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  409 |             log_directory);
      |             ~~~~~~~~~~~~~~

Ticket: #7905.
pull/13926/head
Victor Julien 10 months ago committed by Victor Julien
parent 840d372320
commit 40363f7465
1 changed files with 63 additions and 38 deletions
Show Stats Download Patch File Download Diff File

101
src/output-filestore.c
Unescape Escape View File

@ -1,4 +1,4 @@
/* Copyright (C) 2018-2022 Open Information Security Foundation
/* Copyright (C) 2018-2025 Open Information Security Foundation
*
* You can copy, redistribute or modify this Program under the terms of
* the GNU General Public License version 2 as published by the Free
@ -132,26 +132,35 @@ static void OutputFilestoreFinalizeFiles(ThreadVars *tv, const OutputFilestoreLo
PrintHexString(sha256string, sizeof(sha256string), ff->sha256,
sizeof(ff->sha256));
/* construct tmp file path */
char tmp_filename[PATH_MAX] = "";
snprintf(tmp_filename, sizeof(tmp_filename), "%s/file.%u", ctx->tmpdir,
ff->file_store_id);
char tmp_filepath[PATH_MAX] = "";
snprintf(tmp_filename, sizeof(tmp_filename), "file.%u", ff->file_store_id);
if (PathMerge(tmp_filepath, sizeof(tmp_filepath), ctx->tmpdir, tmp_filename) < 0)
return;
/* construct final path */
char final_sha_short[3];
snprintf(final_sha_short, sizeof(final_sha_short), "%c%c", sha256string[0], sha256string[1]);
char final_sha_portion[PATH_MAX];
if (PathMerge(final_sha_portion, sizeof(final_sha_portion), final_sha_short, sha256string) < 0)
return;
char final_filename[PATH_MAX] = "";
snprintf(final_filename, sizeof(final_filename), "%s/%c%c/%s",
ctx->prefix, sha256string[0], sha256string[1], sha256string);
if (PathMerge(final_filename, sizeof(final_filename), ctx->prefix, final_sha_portion) < 0)
return;
if (SCPathExists(final_filename)) {
OutputFilestoreUpdateFileTime(tmp_filename, final_filename);
if (unlink(tmp_filename) != 0) {
OutputFilestoreUpdateFileTime(tmp_filepath, final_filename);
if (unlink(tmp_filepath) != 0) {
StatsIncr(tv, oft->fs_error_counter);
WARN_ONCE(WOT_UNLINK, "Failed to remove temporary file %s: %s", tmp_filename,
WARN_ONCE(WOT_UNLINK, "Failed to remove temporary file %s: %s", tmp_filepath,
strerror(errno));
}
} else if (rename(tmp_filename, final_filename) != 0) {
} else if (rename(tmp_filepath, final_filename) != 0) {
StatsIncr(tv, oft->fs_error_counter);
WARN_ONCE(WOT_RENAME, "Failed to rename %s to %s: %s", tmp_filename, final_filename,
WARN_ONCE(WOT_RENAME, "Failed to rename %s to %s: %s", tmp_filepath, final_filename,
strerror(errno));
if (unlink(tmp_filename) != 0) {
if (unlink(tmp_filepath) != 0) {
/* Just increment, don't log as has_fs_errors would
* already be set above. */
StatsIncr(tv, oft->fs_error_counter);
@ -160,24 +169,28 @@ static void OutputFilestoreFinalizeFiles(ThreadVars *tv, const OutputFilestoreLo
}
if (ctx->fileinfo) {
char js_metadata_filename[PATH_MAX];
if (snprintf(js_metadata_filename, sizeof(js_metadata_filename), "%s.%" PRIuMAX ".%u.json",
final_filename, (uintmax_t)SCTIME_SECS(p->ts),
ff->file_store_id) == (int)sizeof(js_metadata_filename)) {
char js_metadata_filename_suffix[PATH_MAX];
if (snprintf(js_metadata_filename_suffix, sizeof(js_metadata_filename_suffix),
".%" PRIuMAX ".%u.json", (uintmax_t)SCTIME_SECS(p->ts),
ff->file_store_id) == (int)sizeof(js_metadata_filename_suffix)) {
WARN_ONCE(WOT_SNPRINTF, "Failed to write file info record. Output filename truncated.");
} else {
SCJsonBuilder *js_fileinfo =
JsonBuildFileInfoRecord(p, ff, tx, tx_id, true, dir, ctx->xff_cfg, NULL);
if (likely(js_fileinfo != NULL)) {
SCJbClose(js_fileinfo);
FILE *out = fopen(js_metadata_filename, "w");
if (out != NULL) {
size_t js_len = SCJbLen(js_fileinfo);
fwrite(SCJbPtr(js_fileinfo), js_len, 1, out);
fclose(out);
}
SCJbFree(js_fileinfo);
return;
}
char js_metadata_filename[PATH_MAX];
strlcpy(js_metadata_filename, final_filename, sizeof(js_metadata_filename));
strlcat(js_metadata_filename, js_metadata_filename_suffix, sizeof(js_metadata_filename));
SCJsonBuilder *js_fileinfo =
JsonBuildFileInfoRecord(p, ff, tx, tx_id, true, dir, ctx->xff_cfg, NULL);
if (likely(js_fileinfo != NULL)) {
SCJbClose(js_fileinfo);
FILE *out = fopen(js_metadata_filename, "w");
if (out != NULL) {
size_t js_len = SCJbLen(js_fileinfo);
fwrite(SCJbPtr(js_fileinfo), js_len, 1, out);
fclose(out);
}
SCJbFree(js_fileinfo);
}
}
}
@ -195,7 +208,12 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet
SCLogDebug("ff %p, data %p, data_len %u", ff, data, data_len);
if (flags & OUTPUT_FILEDATA_FLAG_OPEN) {
snprintf(filename, sizeof(filename), "%s/file.%u", ctx->tmpdir, ff->file_store_id);
/* construct tmp file path */
char tmp_filename[PATH_MAX] = "";
snprintf(tmp_filename, sizeof(tmp_filename), "file.%u", ff->file_store_id);
if (PathMerge(filename, sizeof(filename), ctx->tmpdir, tmp_filename) < 0)
return -1;
file_fd = open(filename, O_CREAT | O_TRUNC | O_NOFOLLOW | O_WRONLY,
0644);
if (file_fd == -1) {
@ -216,7 +234,11 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet
/* we can get called with a NULL ffd when we need to close */
} else if (data != NULL) {
if (ff->fd == -1) {
snprintf(filename, sizeof(filename), "%s/file.%u", ctx->tmpdir, ff->file_store_id);
/* construct tmp file path */
char tmp_filename[PATH_MAX] = "";
snprintf(tmp_filename, sizeof(tmp_filename), "file.%u", ff->file_store_id);
if (PathMerge(filename, sizeof(filename), ctx->tmpdir, tmp_filename) < 0)
return -1;
file_fd = open(filename, O_APPEND | O_NOFOLLOW | O_WRONLY);
if (file_fd == -1) {
StatsIncr(tv, aft->fs_error_counter);
@ -232,7 +254,11 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet
if (file_fd != -1) {
ssize_t r = write(file_fd, (const void *)data, (size_t)data_len);
if (r == -1) {
snprintf(filename, sizeof(filename), "%s/file.%u", ctx->tmpdir, ff->file_store_id);
/* construct tmp file path */
char tmp_filename[PATH_MAX] = "";
snprintf(tmp_filename, sizeof(tmp_filename), "file.%u", ff->file_store_id);
if (PathMerge(filename, sizeof(filename), ctx->tmpdir, tmp_filename) < 0)
return -1;
StatsIncr(tv, aft->fs_error_counter);
WARN_ONCE(WOT_WRITE, "Filestore (v2) failed to write to %s: %s", filename,
strerror(errno));
@ -321,7 +347,7 @@ static void GetLogDirectory(const SCConfNode *conf, char *out, size_t out_size)
strlcpy(out, log_base_dir, out_size);
} else {
const char *default_log_prefix = SCConfigGetLogDirectory();
snprintf(out, out_size, "%s/%s", default_log_prefix, log_base_dir);
PathMerge(out, out_size, default_log_prefix, log_base_dir);
}
}
@ -338,9 +364,10 @@ static bool InitFilestoreDirectory(const char *dir)
}
for (int i = 0; i <= dir_count; i++) {
char hex[3];
snprintf(hex, sizeof(hex), "%02x", i);
char leaf[PATH_MAX];
int n = snprintf(leaf, sizeof(leaf), "%s/%02x", dir, i);
if (n < 0 || n >= PATH_MAX) {
if (PathMerge(leaf, sizeof(leaf), dir, hex) < 0) {
SCLogError("Filestore (v2) failed to create leaf directory: "
"path too long");
return false;
@ -357,8 +384,7 @@ static bool InitFilestoreDirectory(const char *dir)
/* Make sure the tmp directory exists. */
char tmpdir[PATH_MAX];
int n = snprintf(tmpdir, sizeof(tmpdir), "%s/tmp", dir);
if (n < 0 || n >= PATH_MAX) {
if (PathMerge(tmpdir, sizeof(tmpdir), dir, "tmp") < 0) {
SCLogError("Filestore (v2) failed to create tmp directory: path too long");
return false;
}
@ -405,9 +431,8 @@ static OutputInitResult OutputFilestoreLogInitCtx(SCConfNode *conf)
}
strlcpy(ctx->prefix, log_directory, sizeof(ctx->prefix));
int written = snprintf(ctx->tmpdir, sizeof(ctx->tmpdir) - 1, "%s/tmp",
log_directory);
if (written == sizeof(ctx->tmpdir)) {
if (PathMerge(ctx->tmpdir, sizeof(ctx->tmpdir), log_directory, "tmp") < 0) {
SCLogError("File-store output directory overflow.");
SCFree(ctx);
return result;

Write Preview
Loading…
Cancel
Save