Skip to content

Commit

Permalink
datasets: don't allow absolute or paths with directory traversal
Browse files Browse the repository at this point in the history
For dataset filenames coming from rules, do not allow filenames that
are absolute or contain a directory traversal with "..". This prevents
datasets from escaping the define data-directory which may allow a bad
rule to overwrite any file that Suricata has permission to write to.

Add a new configuration option,
"datasets.rules.allow-absolute-filenames" to allow absolute filenames
in dataset rules. This will be a way to revert back to the pre 6.0.13
behavior where save/state rules could use any filename.

Ticket: OISF#6118
  • Loading branch information
jasonish authored and victorjulien committed Jun 14, 2023
1 parent 4a97461 commit fd79b33
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 2 deletions.
16 changes: 14 additions & 2 deletions src/detect-dataset.c
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,20 @@ static int SetupSavePath(const DetectEngineCtx *de_ctx,
{
SCLogDebug("save %s", save);

if (PathIsAbsolute(save)) {
return 0;
int allow_absolute = 0;
(void)ConfGetBool("datasets.rules.allow-absolute-filenames", &allow_absolute);
if (allow_absolute) {
SCLogNotice("Allowing absolute filename for dataset rule: %s", save);
} else {
if (PathIsAbsolute(save)) {
SCLogError("Absolute paths not allowed: %s", save);
return -1;
}

if (SCPathContainsTraversal(save)) {
SCLogError("Directory traversals not allowed: %s", save);
return -1;
}
}

// data dir
Expand Down
17 changes: 17 additions & 0 deletions src/util-path.c
Original file line number Diff line number Diff line change
Expand Up @@ -246,3 +246,20 @@ const char *SCBasename(const char *path)

return final + 1;
}

/**
* \brief Check for directory traversal
*
* \param path The path string to check for traversal
*
* \retval true if directory traversal is found, otherwise false
*/
bool SCPathContainsTraversal(const char *path)
{
#ifdef OS_WIN32
const char *pattern = "..\\";
#else
const char *pattern = "../";
#endif
return strstr(path, pattern) != NULL;
}
1 change: 1 addition & 0 deletions src/util-path.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,6 @@ bool SCIsRegularDirectory(const struct dirent *const dir_entry);
bool SCIsRegularFile(const struct dirent *const dir_entry);
char *SCRealPath(const char *path, char *resolved_path);
const char *SCBasename(const char *path);
bool SCPathContainsTraversal(const char *path);

#endif /* __UTIL_PATH_H__ */
6 changes: 6 additions & 0 deletions suricata.yaml.in
Original file line number Diff line number Diff line change
Expand Up @@ -1158,6 +1158,12 @@ asn1-max-frames: 256
# defaults:
# memcap: 100mb
# hashsize: 2048
#
# rules:
# # Set to true to allow absolute filenames and filenames that use
# # ".." components to reference parent directories in rules that specify
# # their filenames.
# #allow-absolute-filenames: false

##############################################################################
##
Expand Down

0 comments on commit fd79b33

Please sign in to comment.