Skip to content

Commit 6097d82

Browse files
author
AztecBot
committed
fix: bb shell injections
1 parent 34eabaa commit 6097d82

File tree

5 files changed

+118
-24
lines changed

5 files changed

+118
-24
lines changed

barretenberg/cpp/src/barretenberg/api/exec_pipe.hpp

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,15 @@
55
#include <cstring>
66
#include <memory>
77
#include <vector>
8+
#include <filesystem>
9+
#include <random>
10+
#include <string>
11+
#include <sstream>
812

913
namespace bb {
10-
inline std::vector<uint8_t> exec_pipe([[maybe_unused]] const std::string& command)
14+
15+
// Unsafe version - renamed to make it clear this is dangerous
16+
inline std::vector<uint8_t> exec_pipe_unsafe([[maybe_unused]] const std::string& command)
1117
{
1218
#ifdef __wasm__
1319
throw_or_abort("Can't use popen() in wasm! Implement this functionality natively.");
@@ -27,4 +33,101 @@ inline std::vector<uint8_t> exec_pipe([[maybe_unused]] const std::string& comman
2733
return output;
2834
#endif
2935
}
36+
37+
// Tag type to mark string literals as safe
38+
template<size_t N>
39+
struct literal_string {
40+
constexpr literal_string(const char (&str)[N]) {
41+
std::copy_n(str, N, value);
42+
}
43+
char value[N];
44+
};
45+
46+
// Helper to check if a type is a literal_string
47+
template<typename T>
48+
struct is_literal_string : std::false_type {};
49+
50+
template<size_t N>
51+
struct is_literal_string<literal_string<N>> : std::true_type {};
52+
53+
template<typename T>
54+
inline constexpr bool is_literal_string_v = is_literal_string<T>::value;
55+
56+
// Concept to ensure arguments are safe for command construction
57+
template<typename T>
58+
concept SafeCommandArg = std::is_arithmetic_v<std::decay_t<T>> || is_literal_string_v<std::decay_t<T>>;
59+
60+
// Safe exec_pipe_safe that builds command from literal strings and numbers
61+
template<SafeCommandArg... Args>
62+
inline std::vector<uint8_t> exec_pipe_safe(Args... args) {
63+
std::ostringstream command;
64+
auto append = [&command](auto&& arg) {
65+
using T = std::decay_t<decltype(arg)>;
66+
if constexpr (std::is_arithmetic_v<T>) {
67+
command << arg;
68+
} else {
69+
command << arg.value;
70+
}
71+
};
72+
(append(args), ...);
73+
return exec_pipe_unsafe(command.str());
74+
}
75+
76+
// RAII class for creating temporary symlinks to safely pass file paths to shell commands
77+
class SafePathSymlink {
78+
std::filesystem::path symlink_path;
79+
bool created = false;
80+
81+
static uint64_t get_random_suffix() {
82+
// Thread-local RNG for performance
83+
static thread_local std::random_device rd;
84+
static thread_local std::mt19937_64 gen(rd());
85+
static thread_local std::uniform_int_distribution<uint64_t> dis(100000, 99999999);
86+
return dis(gen);
87+
}
88+
89+
public:
90+
explicit SafePathSymlink(const std::string& target_path) {
91+
// Generate random temporary symlink name
92+
std::filesystem::path temp_dir = std::filesystem::temp_directory_path();
93+
std::string random_name = "bb_safe_" + std::to_string(get_random_suffix());
94+
symlink_path = temp_dir / random_name;
95+
96+
// Create symlink
97+
std::error_code ec;
98+
std::filesystem::create_symlink(target_path, symlink_path, ec);
99+
if (ec) {
100+
throw_or_abort("Failed to create temporary symlink: " + ec.message());
101+
}
102+
created = true;
103+
}
104+
105+
~SafePathSymlink() {
106+
if (created) {
107+
std::error_code ec;
108+
std::filesystem::remove(symlink_path, ec);
109+
}
110+
}
111+
112+
// Delete copy and move operations - only needed as stack variable in exec_pipe_with_path
113+
SafePathSymlink(const SafePathSymlink&) = delete;
114+
SafePathSymlink& operator=(const SafePathSymlink&) = delete;
115+
SafePathSymlink(SafePathSymlink&&) = delete;
116+
SafePathSymlink& operator=(SafePathSymlink&&) = delete;
117+
118+
std::string path() const {
119+
return symlink_path.string();
120+
}
121+
};
122+
123+
// Helper function to execute a command with a safe file path (requires literal string prefixes)
124+
template<size_t N1, size_t N2 = 1>
125+
inline std::vector<uint8_t> exec_pipe_with_path(const char (&command_prefix)[N1],
126+
const std::string& file_path,
127+
const char (&command_suffix)[N2] = "") {
128+
SafePathSymlink safe_path(file_path);
129+
std::string command = std::string(command_prefix) + safe_path.path() + std::string(command_suffix);
130+
return exec_pipe_unsafe(command);
131+
}
132+
30133
} // namespace bb

barretenberg/cpp/src/barretenberg/api/get_bytecode.hpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@
99
*/
1010
inline std::vector<uint8_t> gunzip(const std::string& path)
1111
{
12-
std::string command = "gunzip -c \"" + path + "\"";
13-
return bb::exec_pipe(command);
12+
return bb::exec_pipe_with_path("gunzip -c ", path);
1413
}
1514

1615
inline std::vector<uint8_t> get_bytecode(const std::string& bytecodePath)
@@ -21,8 +20,7 @@ inline std::vector<uint8_t> get_bytecode(const std::string& bytecodePath)
2120
std::filesystem::path filePath = bytecodePath;
2221
if (filePath.extension() == ".json") {
2322
// Try reading json files as if they are a Nargo build artifact
24-
std::string command = "jq -r '.bytecode' \"" + bytecodePath + "\" | base64 -d | gunzip -c";
25-
return bb::exec_pipe(command);
23+
return bb::exec_pipe_with_path("jq -r '.bytecode' ", bytecodePath, " | base64 -d | gunzip -c");
2624
}
2725

2826
// For other extensions, assume file is a raw ACIR program

barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,11 @@ class AcirIntegrationTest : public ::testing::Test {
2121
std::filesystem::path filePath = bytecodePath;
2222
if (filePath.extension() == ".json") {
2323
// Try reading json files as if they are a Nargo build artifact
24-
std::string command = "jq -r '.bytecode' \"" + bytecodePath + "\" | base64 -d | gunzip -c";
25-
return exec_pipe(command);
24+
return exec_pipe_with_path("jq -r '.bytecode' ", bytecodePath, " | base64 -d | gunzip -c");
2625
}
2726

2827
// For other extensions, assume file is a raw ACIR program
29-
std::string command = "gunzip -c \"" + bytecodePath + "\"";
30-
return exec_pipe(command);
28+
return exec_pipe_with_path("gunzip -c ", bytecodePath);
3129
}
3230

3331
// Function to check if a file exists

barretenberg/cpp/src/barretenberg/srs/factories/get_bn254_crs.cpp

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,10 @@ std::vector<uint8_t> download_bn254_g1_data(size_t num_points)
88
{
99
size_t g1_end = num_points * sizeof(bb::g1::affine_element) - 1;
1010

11-
std::string url = "https://crs.aztec.network/g1.dat";
12-
13-
// IMPORTANT: this currently uses a shell, DO NOT let user-controlled strings here.
14-
std::string command = "curl -H \"Range: bytes=0-" + std::to_string(g1_end) + "\" '" + url + "'";
15-
16-
auto data = bb::exec_pipe(command);
11+
// Safe command construction with numeric interpolation and hardcoded URL
12+
auto data = bb::exec_pipe_safe(literal_string("curl -H \"Range: bytes=0-"),
13+
g1_end,
14+
literal_string("\" 'https://crs.aztec.network/g1.dat'"));
1715
// Header + num_points * sizeof point.
1816
if (data.size() < g1_end) {
1917
throw_or_abort("Failed to download g1 data.");
@@ -24,10 +22,8 @@ std::vector<uint8_t> download_bn254_g1_data(size_t num_points)
2422

2523
std::vector<uint8_t> download_bn254_g2_data()
2624
{
27-
std::string url = "https://crs.aztec.network/g2.dat";
28-
// IMPORTANT: this currently uses a shell, DO NOT let user-controlled strings here.
29-
std::string command = "curl '" + url + "'";
30-
return bb::exec_pipe(command);
25+
// Safe command with hardcoded URL - using exec_pipe_safe with single literal
26+
return bb::exec_pipe_safe(literal_string("curl 'https://crs.aztec.network/g2.dat'"));
3127
}
3228
} // namespace
3329

barretenberg/cpp/src/barretenberg/srs/factories/get_grumpkin_crs.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,11 @@ namespace {
99
std::vector<uint8_t> download_grumpkin_g1_data(size_t num_points)
1010
{
1111
size_t g1_end = num_points * sizeof(bb::curve::Grumpkin::AffineElement) - 1;
12-
std::string url = "https://crs.aztec.network/grumpkin_g1.dat";
1312

14-
// IMPORTANT: this currently uses a shell, DO NOT let user-controlled strings here.
15-
std::string command = "curl -s -H \"Range: bytes=0-" + std::to_string(g1_end) + "\" '" + url + "'";
16-
17-
auto data = bb::exec_pipe(command);
13+
// Safe command construction with numeric interpolation and hardcoded URL
14+
auto data = bb::exec_pipe_safe(literal_string("curl -s -H \"Range: bytes=0-"),
15+
g1_end,
16+
literal_string("\" 'https://crs.aztec.network/grumpkin_g1.dat'"));
1817
if (data.size() < g1_end) {
1918
THROW std::runtime_error("Failed to download grumpkin g1 data.");
2019
}

0 commit comments

Comments
 (0)