Skip to content

Commit bfd05d0

Browse files
committed
Fix bugs A, B, C, D, E, F
A: file_read rejects non-number/negative size before the size_t cast (was: std::length_error escaping as Fatal Error). B: json_stringify tracks visited lists/dicts, errors on cycles (was: stack overflow segfault). C: json_parse parse_val takes a depth param, caps at 1000 (was: stack overflow segfault on deep input). D: std_db_query_rows bind loop uses a separate bind counter (was: explicit db handle shifted all later params, silent null). E: Chunk::deserialize / ObjFunction::deserialize take a depth param, cap at 1000 (was: stack overflow segfault on nested .shbc). F: OP_CLOSURE checks upvalue index against the frame stack window (is_local) or the enclosing closure's upvalue count (was: OOB read / wild capture segfault).
1 parent a65a5b0 commit bfd05d0

14 files changed

Lines changed: 402 additions & 88 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,11 @@ jobs:
9090
with open(f, encoding="utf-8") as fh:
9191
src = fh.read()
9292
once = subprocess.run([exe, "fmt", "--stdin"], input=src,
93-
capture_output=True, text=True).stdout
93+
capture_output=True, text=True,
94+
encoding="utf-8").stdout
9495
twice = subprocess.run([exe, "fmt", "--stdin"], input=once,
95-
capture_output=True, text=True).stdout
96+
capture_output=True, text=True,
97+
encoding="utf-8").stdout
9698
if once != twice:
9799
print(f"NOT IDEMPOTENT: {f}")
98100
bad += 1

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Build artifacts
22
build/
33
build_cpp/
4+
build_asan/
45
package/
56
dist/
67
bin/

‎shell_lite/chunk.cpp‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "objects.hpp"
44
#include "vm.hpp"
55
#include <iostream>
6+
#include <memory>
67
namespace shell_lite {
78

89
int Chunk::add_constant(Value value) {
@@ -75,31 +76,31 @@ void Chunk::serialize(std::ostream& out) const {
7576
}
7677
}
7778

78-
Chunk* Chunk::deserialize(std::istream& in, GCArena& arena) {
79-
auto* c = new Chunk();
79+
Chunk* Chunk::deserialize(std::istream& in, GCArena& arena, int depth) {
80+
auto c = std::make_unique<Chunk>();
81+
if (depth > 1000) {
82+
throw std::runtime_error("Function nesting too deep in bytecode");
83+
}
8084
uint32_t magic = 0;
8185
read_bin(in, magic);
8286
if (!in || magic != 0x43424853) {
83-
delete c;
8487
throw std::runtime_error("Invalid SHBC bytecode: missing magic header");
8588
}
8689
uint32_t version = 0;
8790
read_bin(in, version);
8891
if (!in || version > 0x00010000) {
89-
delete c;
9092
throw std::runtime_error("Unsupported SHBC bytecode version");
9193
}
9294

9395
uint32_t const_count = 0;
9496
read_bin(in, const_count);
9597
if (!in || const_count > 1000000) {
96-
delete c;
9798
throw std::runtime_error("Invalid constant count in bytecode");
9899
}
99100
for (uint32_t i = 0; i < const_count; ++i) {
100101
uint8_t tag = 0;
101102
read_bin(in, tag);
102-
if (!in) { delete c; throw std::runtime_error("Unexpected EOF in bytecode constants"); }
103+
if (!in) { throw std::runtime_error("Unexpected EOF in bytecode constants"); }
103104
if (tag == 0x00) {
104105
c->constants.push_back(Value());
105106
} else if (tag == 0x01) {
@@ -110,35 +111,32 @@ Chunk* Chunk::deserialize(std::istream& in, GCArena& arena) {
110111
c->constants.push_back(Value(d));
111112
} else if (tag == 0x03) {
112113
uint32_t len = 0; read_bin(in, len);
113-
if (!in || len > 10000000) { delete c; throw std::runtime_error("Invalid string length in bytecode"); }
114+
if (!in || len > 10000000) { throw std::runtime_error("Invalid string length in bytecode"); }
114115
std::string s(len, '\0');
115116
in.read(&s[0], len);
116-
if (!in) { delete c; throw std::runtime_error("Unexpected EOF reading string constant"); }
117+
if (!in) { throw std::runtime_error("Unexpected EOF reading string constant"); }
117118
c->constants.push_back(Value(arena.allocate_string(s)));
118119
} else if (tag == 0x04) {
119-
c->constants.push_back(Value(ObjFunction::deserialize(in, arena)));
120+
c->constants.push_back(Value(ObjFunction::deserialize(in, arena, depth + 1)));
120121
} else {
121-
delete c;
122122
throw std::runtime_error("Invalid constant type tag in SHBC");
123123
}
124124
}
125125

126126
uint32_t code_size = 0;
127127
read_bin(in, code_size);
128128
if (!in || code_size > 10000000) {
129-
delete c;
130129
throw std::runtime_error("Invalid code size in bytecode");
131130
}
132131
c->code.resize(code_size);
133132
if (code_size > 0) {
134133
in.read(reinterpret_cast<char*>(c->code.data()), code_size);
135-
if (!in) { delete c; throw std::runtime_error("Unexpected EOF reading bytecode"); }
134+
if (!in) { throw std::runtime_error("Unexpected EOF reading bytecode"); }
136135
}
137136

138137
uint32_t loc_count = 0;
139138
read_bin(in, loc_count);
140139
if (!in || loc_count > 1000000) {
141-
delete c;
142140
throw std::runtime_error("Invalid location count in bytecode");
143141
}
144142
for (uint32_t i = 0; i < loc_count; ++i) {
@@ -148,10 +146,9 @@ Chunk* Chunk::deserialize(std::istream& in, GCArena& arena) {
148146
}
149147

150148
if (!c->verify()) {
151-
delete c;
152149
throw std::runtime_error("Bytecode verification failed for chunk");
153150
}
154-
return c;
151+
return c.release();
155152
}
156153

157154
bool Chunk::verify() const {

‎shell_lite/chunk.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ struct Chunk {
128128
Chunk *clone(GCArena &target,
129129
std::unordered_map<GCObject *, GCObject *> &clones);
130130
void serialize(std::ostream& out) const;
131-
static Chunk* deserialize(std::istream& in, GCArena& arena);
131+
static Chunk* deserialize(std::istream& in, GCArena& arena, int depth = 0);
132132
bool verify() const;
133133
};
134134

‎shell_lite/formatter.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -455,8 +455,6 @@ int Formatter::expr_end(Node *node) {
455455
return e;
456456
}
457457

458-
// last line of a statement by matching brackets in the source
459-
// ast nodes dont cover closers sitting on their own line
460458
int Formatter::bracket_end(int from_line, int bound_line) const {
461459
if (from_line < 1 || (size_t)from_line > line_offsets_.size())
462460
return from_line;
@@ -490,8 +488,6 @@ int Formatter::bracket_end(int from_line, int bound_line) const {
490488
if (depth == 0 && has_code)
491489
candidate = line;
492490
has_code = false;
493-
// a depth zero line followed by same or lower indent ends the statement
494-
// continuations are deeper indented or sit inside brackets
495491
if (depth == 0 && line + 1 < last && line_indent(line + 1) <= base_indent)
496492
stop = true;
497493
};

‎shell_lite/modules/module_core.cpp‎

Lines changed: 59 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#include <sstream>
99
#include <cmath>
1010
#include <functional>
11+
#include <set>
12+
#include <unordered_set>
1113
#include <unordered_map>
1214
#include <cstdint>
1315
#include <cstdio>
@@ -881,7 +883,27 @@ void register_stdlib_core(VM *vm) {
881883
Value v = vm->peek(arg_count - 1);
882884
int indent_size = (arg_count >= 2) ? (int)vm->peek(arg_count - 2).as_number() : 0;
883885

886+
std::unordered_set<const void*> seen;
887+
auto json_escape = [](const std::string& s) -> std::string {
888+
std::string res = "\"";
889+
for (char c : s) {
890+
switch (c) {
891+
case '"': res += "\\\""; break;
892+
case '\\': res += "\\\\"; break;
893+
case '\b': res += "\\b"; break;
894+
case '\f': res += "\\f"; break;
895+
case '\n': res += "\\n"; break;
896+
case '\r': res += "\\r"; break;
897+
case '\t': res += "\\t"; break;
898+
default: res += c; break;
899+
}
900+
}
901+
res += "\"";
902+
return res;
903+
};
884904
std::function<std::string(Value, int)> stringify = [&](Value val, int depth) -> std::string {
905+
if (depth > 1000)
906+
throw std::runtime_error("json_stringify: nesting too deep");
885907
if (val.is_null()) return "null";
886908
if (val.is_bool()) return val.as_bool() ? "true" : "false";
887909
if (val.is_number()) {
@@ -893,80 +915,75 @@ void register_stdlib_core(VM *vm) {
893915
ss << num;
894916
return ss.str();
895917
}
896-
if (val.is_string()) {
897-
std::string s = val.as_string();
898-
std::string res = "\"";
899-
for (char c : s) {
900-
switch (c) {
901-
case '"': res += "\\\""; break;
902-
case '\\': res += "\\\\"; break;
903-
case '\b': res += "\\b"; break;
904-
case '\f': res += "\\f"; break;
905-
case '\n': res += "\\n"; break;
906-
case '\r': res += "\\r"; break;
907-
case '\t': res += "\\t"; break;
908-
default: res += c; break;
909-
}
910-
}
911-
res += "\"";
912-
return res;
913-
}
918+
if (val.is_string()) return json_escape(val.as_string());
914919
if (val.is_list()) {
915920
auto *list = static_cast<ObjList*>(val.get_obj());
916-
if (list->elements.empty()) return "[]";
917-
if (indent_size > 0) {
921+
if (!seen.insert(list).second)
922+
throw std::runtime_error("json_stringify: cyclic structure");
923+
std::string res;
924+
if (list->elements.empty()) res = "[]";
925+
else if (indent_size > 0) {
918926
std::string indent_str(depth * indent_size, ' ');
919927
std::string inner_indent((depth + 1) * indent_size, ' ');
920-
std::string res = "[\n";
928+
res = "[\n";
921929
for (size_t i = 0; i < list->elements.size(); ++i) {
922930
if (i > 0) res += ",\n";
923931
res += inner_indent + stringify(list->elements[i], depth + 1);
924932
}
925933
res += "\n" + indent_str + "]";
926-
return res;
927934
} else {
928-
std::string res = "[";
935+
res = "[";
929936
for (size_t i = 0; i < list->elements.size(); ++i) {
930937
if (i > 0) res += ", ";
931-
res += stringify(list->elements[i], 0);
938+
res += stringify(list->elements[i], depth + 1);
932939
}
933940
res += "]";
934-
return res;
935941
}
942+
seen.erase(list);
943+
return res;
936944
}
937945
if (val.is_dict()) {
938946
auto *dict = static_cast<ObjDict*>(val.get_obj());
939-
if (dict->elements.empty()) return "{}";
940-
if (indent_size > 0) {
947+
if (!seen.insert(dict).second)
948+
throw std::runtime_error("json_stringify: cyclic structure");
949+
std::string res;
950+
if (dict->elements.empty()) res = "{}";
951+
else if (indent_size > 0) {
941952
std::string indent_str(depth * indent_size, ' ');
942953
std::string inner_indent((depth + 1) * indent_size, ' ');
943-
std::string res = "{\n";
954+
res = "{\n";
944955
bool first = true;
945956
for (auto &pair : dict->elements) {
946957
if (!first) res += ",\n";
947958
first = false;
948-
res += inner_indent + stringify(Value(vm->arena().allocate_string(pair.first)), 0);
959+
res += inner_indent + json_escape(pair.first);
949960
res += ": " + stringify(pair.second, depth + 1);
950961
}
951962
res += "\n" + indent_str + "}";
952-
return res;
953963
} else {
954-
std::string res = "{";
964+
res = "{";
955965
bool first = true;
956966
for (auto &pair : dict->elements) {
957967
if (!first) res += ", ";
958968
first = false;
959-
res += stringify(Value(vm->arena().allocate_string(pair.first)), 0);
969+
res += json_escape(pair.first);
960970
res += ": ";
961-
res += stringify(pair.second, 0);
971+
res += stringify(pair.second, depth + 1);
962972
}
963973
res += "}";
964-
return res;
965974
}
975+
seen.erase(dict);
976+
return res;
966977
}
967978
return "\"" + val.to_string() + "\"";
968979
};
969-
return Value(vm->arena().allocate_string(stringify(v, 0)));
980+
try {
981+
return Value(vm->arena().allocate_string(stringify(v, 0)));
982+
} catch (const std::exception& e) {
983+
vm->has_error = true;
984+
vm->error_value = Value(vm->arena().allocate_string(e.what()));
985+
return Value();
986+
}
970987
};
971988

972989
NativeRegistry::register_builtin(vm, "json_stringify", -1, json_stringify_fn);
@@ -1020,9 +1037,10 @@ void register_stdlib_core(VM *vm) {
10201037
return out;
10211038
};
10221039

1023-
std::function<Value()> parse_val = [&]() -> Value {
1040+
std::function<Value(int)> parse_val = [&](int depth) -> Value {
10241041
skip_ws();
10251042
if (pos >= s.size()) { error_out = true; return Value(); }
1043+
if (depth > 1000) { error_out = true; return Value(); }
10261044
char c = s[pos];
10271045
if (c == 'n' && s.substr(pos, 4) == "null") {
10281046
pos += 4;
@@ -1046,7 +1064,8 @@ void register_stdlib_core(VM *vm) {
10461064
skip_ws();
10471065
if (match_c(']')) return Value(list);
10481066
while (true) {
1049-
Value elem = parse_val();
1067+
Value elem = parse_val(depth + 1);
1068+
if (error_out) break;
10501069
list->elements.push_back(elem);
10511070
skip_ws();
10521071
if (match_c(']')) break;
@@ -1065,7 +1084,8 @@ void register_stdlib_core(VM *vm) {
10651084
std::string key = parse_str();
10661085
skip_ws();
10671086
if (!match_c(':')) { error_out = true; break; }
1068-
Value v = parse_val();
1087+
Value v = parse_val(depth + 1);
1088+
if (error_out) break;
10691089
dict->elements[key] = v;
10701090
skip_ws();
10711091
if (match_c('}')) break;
@@ -1098,7 +1118,7 @@ void register_stdlib_core(VM *vm) {
10981118
return Value();
10991119
};
11001120

1101-
Value result = parse_val();
1121+
Value result = parse_val(0);
11021122
skip_ws();
11031123
if (pos < s.size()) error_out = true;
11041124
return result;

‎shell_lite/modules/module_db.cpp‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,16 @@ void register_stdlib_db(VM* vm) {
120120
vm->error_value = Value(vm->arena().allocate_string("Database not open. Call db_open() or connect_database() first."));
121121
return Value();
122122
}
123-
std::string query = vm->peek(arg_count - 1).to_string();
123+
int query_peek = -1;
124+
for (int i = arg_count - 1; i >= 0; --i) {
125+
if (!vm->peek(i).is_database()) { query_peek = i; break; }
126+
}
127+
if (query_peek < 0) {
128+
vm->has_error = true;
129+
vm->error_value = Value(vm->arena().allocate_string("db_query expects a query string"));
130+
return Value();
131+
}
132+
std::string query = vm->peek(query_peek).to_string();
124133

125134
sqlite3_stmt* stmt = nullptr;
126135
if (sqlite3_prepare_v2(conn, query.c_str(), -1, &stmt, nullptr) != SQLITE_OK) {
@@ -129,13 +138,17 @@ void register_stdlib_db(VM* vm) {
129138
return Value();
130139
}
131140

132-
for (int i = 0; i < arg_count - 1; ++i) {
133-
Value v = vm->peek(arg_count - 2 - i);
141+
int bind_idx = 1;
142+
int query_arg = arg_count - 1 - query_peek;
143+
for (int j = 0; j < arg_count; ++j) {
144+
if (j == query_arg) continue;
145+
Value v = vm->peek(arg_count - 1 - j);
134146
if (v.is_database()) continue;
135-
if (v.is_null()) sqlite3_bind_null(stmt, i + 1);
136-
else if (v.is_number()) sqlite3_bind_double(stmt, i + 1, v.as_number());
137-
else if (v.is_bool()) sqlite3_bind_int(stmt, i + 1, v.as_bool() ? 1 : 0);
138-
else sqlite3_bind_text(stmt, i + 1, v.to_string().c_str(), -1, SQLITE_TRANSIENT);
147+
if (v.is_null()) sqlite3_bind_null(stmt, bind_idx);
148+
else if (v.is_number()) sqlite3_bind_double(stmt, bind_idx, v.as_number());
149+
else if (v.is_bool()) sqlite3_bind_int(stmt, bind_idx, v.as_bool() ? 1 : 0);
150+
else sqlite3_bind_text(stmt, bind_idx, v.to_string().c_str(), -1, SQLITE_TRANSIENT);
151+
++bind_idx;
139152
}
140153

141154
auto result_list = vm->arena().allocate<ObjList>();

0 commit comments

Comments
 (0)