Skip to content

Commit

Permalink
Sanitize filenames when using "archive_groupname" (#1383)
Browse files Browse the repository at this point in the history
This feature (used, very possibly, only by me, for debugging) lets me
force OSL to write an archive that contains the oso files and a
serialized version of the shader network.

If no explicit name is given (by the "archive_filename" attribute), it
picks a name based on the shader group name. But that may not be a
safe string for a valid filename. We already eliminated anything
before the last slash, but it turns out that a colon (':') and pipe ('|')
can also really mess things up. So this small change protects against that.

For the actual commands sent to system(), escape special characters
in the strings and enclose any arguments that come from the user in
double quotes. That prevents shenanigans like asking for an archive
filename called "; rm -r *". By enclosing in double quotes, we'll
end up with a badly named filename produced by tar, rather than
possibly an arbitrary command being executed.

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz authored Jul 2, 2021
1 parent 98e3aca commit d6ec4b8
Showing 1 changed file with 18 additions and 8 deletions.
26 changes: 18 additions & 8 deletions src/liboslexec/shadingsys.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2453,8 +2453,15 @@ ShadingSystemImpl::ShaderGroupEnd (ShaderGroup& group)
ustring groupname = group.name();
if (groupname.size() && groupname == m_archive_groupname) {
std::string filename = m_archive_filename.string();
if (! filename.size())
if (! filename.size()) {
filename = OIIO::Filesystem::filename (groupname.string()) + ".tar.gz";
// Transform characters that will ruin our day when passed to
// tar as a filename.
if (OIIO::Strutil::contains(filename, ":"))
filename = OIIO::Strutil::replace(filename, ":", "_");
if (OIIO::Strutil::contains(filename, "|"))
filename = OIIO::Strutil::replace(filename, "|", "_");
}
archive_shadergroup (group, filename);
}

Expand Down Expand Up @@ -3817,26 +3824,29 @@ ShadingSystemImpl::archive_shadergroup (ShaderGroup& group, string_view filename
entries.insert (osoname);
std::string localfile = tmpdir + "/" + osoname;
OIIO::Filesystem::copy (osofile, localfile);
filename_list += " " + osoname;
filename_list += Strutil::fmt::format(" \"{}\"",
Strutil::escape_chars(osoname));
}
}
}

std::string full_filename = Strutil::fmt::format("{}{}",
Strutil::escape_chars(filename),
extension);
if (extension == ".tar" || extension == ".tar.gz" || extension == ".tgz") {
std::string z = Strutil::ends_with (extension, "gz") ? "-z" : "";
std::string cmd = Strutil::sprintf ("tar -c %s -C %s -f %s%s %s",
z, tmpdir, filename, extension,
filename_list);
std::string cmd = Strutil::sprintf("tar -c %s -C \"%s\" -f \"%s\" %s",
z, Strutil::escape_chars(tmpdir),
full_filename, filename_list);
// std::cout << "Command =\n" << cmd << "\n";
if (system (cmd.c_str()) != 0) {
error ("archive_shadergroup: executing tar command failed");
ok = false;
}

} else if (extension == ".zip") {
std::string cmd = Strutil::sprintf ("zip -q %s%s %s",
filename, extension,
filename_list);
std::string cmd = Strutil::sprintf("zip -q \"%s\" %s",
full_filename, filename_list);
// std::cout << "Command =\n" << cmd << "\n";
if (system (cmd.c_str()) != 0) {
error ("archive_shadergroup: executing zip command failed");
Expand Down

0 comments on commit d6ec4b8

Please sign in to comment.