Skip to content

Commit e2f8aec

Browse files
Tcode-MotionTcode-Motion
authored
Merge pull request #17 from Tcode-Motion/security-audit-7368610658854629919
Security audit 7368610658854629919
2 parents b588ced + 3b56be7 commit e2f8aec

3 files changed

Lines changed: 111 additions & 53 deletions

File tree

‎runtime/native_runtime/src/lib.rs‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,11 @@ pub extern "C" fn ts_alloc_map() -> *mut TsValue {
129129

130130
#[no_mangle]
131131
pub extern "C" fn ts_alloc_struct(name: *const c_char) -> *mut TsValue {
132-
let name_str = unsafe { CStr::from_ptr(name).to_string_lossy().into_owned() };
132+
let name_str = if name.is_null() {
133+
String::new()
134+
} else {
135+
unsafe { CStr::from_ptr(name).to_string_lossy().into_owned() }
136+
};
133137
let ts_struct = TsStruct {
134138
name: name_str,
135139
fields: HashMap::new(),
@@ -144,7 +148,11 @@ pub extern "C" fn ts_alloc_struct(name: *const c_char) -> *mut TsValue {
144148

145149
#[no_mangle]
146150
pub extern "C" fn ts_alloc_model(name: *const c_char) -> *mut TsValue {
147-
let name_str = unsafe { CStr::from_ptr(name).to_string_lossy().into_owned() };
151+
let name_str = if name.is_null() {
152+
String::new()
153+
} else {
154+
unsafe { CStr::from_ptr(name).to_string_lossy().into_owned() }
155+
};
148156
let ts_model = TsModel {
149157
name: name_str,
150158
fields: HashMap::new(),
@@ -163,8 +171,16 @@ pub extern "C" fn ts_alloc_enum(
163171
variant: *const c_char,
164172
val_opt: *mut TsValue,
165173
) -> *mut TsValue {
166-
let name_str = unsafe { CStr::from_ptr(name).to_string_lossy().into_owned() };
167-
let variant_str = unsafe { CStr::from_ptr(variant).to_string_lossy().into_owned() };
174+
let name_str = if name.is_null() {
175+
String::new()
176+
} else {
177+
unsafe { CStr::from_ptr(name).to_string_lossy().into_owned() }
178+
};
179+
let variant_str = if variant.is_null() {
180+
String::new()
181+
} else {
182+
unsafe { CStr::from_ptr(variant).to_string_lossy().into_owned() }
183+
};
168184
let ts_enum = TsEnum {
169185
name: name_str,
170186
variant: variant_str,

‎stdlib/src/compress.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,12 @@ pub fn tar_dir(src_dir: &str, dst_file: &str) -> std::io::Result<()> {
266266
pub fn untar_archive(archive_path: &str, dest_dir: &str) -> std::io::Result<()> {
267267
let file = File::open(archive_path)?;
268268
let mut a = tar::Archive::new(file);
269+
270+
// The tar crate's `unpack_in` method already has built-in directory traversal
271+
// protections which prevent absolute paths and parent directory traversals
272+
// from escaping the destination directory. Therefore, we revert the manual
273+
// path validation that caused a regression with uncanonicalized relative paths.
274+
269275
a.unpack(dest_dir)?;
270276
Ok(())
271277
}

‎stdlib/src/sqlite.rs‎

Lines changed: 85 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,15 @@ use indexmap::IndexMap;
33
use std::cell::RefCell;
44
use std::collections::HashMap;
55
use std::rc::Rc;
6+
use std::sync::atomic::{AtomicI64, Ordering};
67
use techscript_runtime::{error::RuntimeError, error::RuntimeErrorKind, value::RuntimeValue};
78

9+
thread_local! {
10+
static CONNECTIONS: RefCell<HashMap<i64, rusqlite::Connection>> = RefCell::new(HashMap::new());
11+
}
12+
13+
static NEXT_ID: AtomicI64 = AtomicI64::new(1);
14+
815
impl StdlibRegistry {
916
pub fn register_sqlite(&mut self) {
1017
let mut exports: HashMap<String, Rc<dyn techscript_runtime::function::Callable>> =
@@ -24,8 +31,9 @@ impl StdlibRegistry {
2431
None,
2532
)
2633
})?;
27-
let ptr = Box::into_raw(Box::new(conn)) as i64;
28-
Ok(RuntimeValue::Int(ptr))
34+
let id = NEXT_ID.fetch_add(1, Ordering::SeqCst);
35+
CONNECTIONS.with(|m| m.borrow_mut().insert(id, conn));
36+
Ok(RuntimeValue::Int(id))
2937
},
3038
}),
3139
);
@@ -36,21 +44,35 @@ impl StdlibRegistry {
3644
name: "execute".to_string(),
3745
arity: 2,
3846
callback: |_ctx, args| {
39-
let ptr = args[0].try_into_int().map_err(|_| {
47+
let id = args[0].try_into_int().map_err(|_| {
4048
RuntimeError::new(
4149
RuntimeErrorKind::InvalidOperation("expected int handle".to_string()),
4250
None,
4351
None,
4452
)
4553
})?;
4654
let sql = args[1].to_string();
47-
let conn = unsafe { &mut *(ptr as *mut rusqlite::Connection) };
48-
conn.execute(&sql, []).map_err(|e| {
49-
RuntimeError::new(
50-
RuntimeErrorKind::InvalidOperation(e.to_string()),
51-
None,
52-
None,
53-
)
55+
56+
CONNECTIONS.with(|m| {
57+
let mut map = m.borrow_mut();
58+
if let Some(conn) = map.get_mut(&id) {
59+
conn.execute(&sql, []).map_err(|e| {
60+
RuntimeError::new(
61+
RuntimeErrorKind::InvalidOperation(e.to_string()),
62+
None,
63+
None,
64+
)
65+
})?;
66+
Ok(())
67+
} else {
68+
Err(RuntimeError::new(
69+
RuntimeErrorKind::InvalidOperation(
70+
"Invalid connection handle".to_string(),
71+
),
72+
None,
73+
None,
74+
))
75+
}
5476
})?;
5577
Ok(RuntimeValue::Null)
5678
},
@@ -63,52 +85,68 @@ impl StdlibRegistry {
6385
name: "query".to_string(),
6486
arity: 2,
6587
callback: |_ctx, args| {
66-
let ptr = args[0].try_into_int().map_err(|_| {
88+
let id = args[0].try_into_int().map_err(|_| {
6789
RuntimeError::new(
6890
RuntimeErrorKind::InvalidOperation("expected int handle".to_string()),
6991
None,
7092
None,
7193
)
7294
})?;
7395
let sql = args[1].to_string();
74-
let conn = unsafe { &mut *(ptr as *mut rusqlite::Connection) };
75-
let mut stmt = conn.prepare(&sql).map_err(|e| {
76-
RuntimeError::new(
77-
RuntimeErrorKind::InvalidOperation(e.to_string()),
78-
None,
79-
None,
80-
)
81-
})?;
82-
let col_count = stmt.column_count();
83-
let col_names: Vec<String> = (0..col_count)
84-
.map(|i| stmt.column_name(i).unwrap_or("?").to_string())
85-
.collect();
86-
let mut rows = Vec::new();
87-
let row_iter = stmt
88-
.query_map([], |row| {
89-
let mut map = IndexMap::new();
90-
for i in 0..col_count {
91-
let name = col_names[i].clone();
92-
let val: String = row.get::<_, String>(i).unwrap_or_default();
93-
map.insert(name, RuntimeValue::Str(val));
96+
97+
let rows = CONNECTIONS.with(|m| {
98+
let mut map = m.borrow_mut();
99+
if let Some(conn) = map.get_mut(&id) {
100+
let mut stmt = conn.prepare(&sql).map_err(|e| {
101+
RuntimeError::new(
102+
RuntimeErrorKind::InvalidOperation(e.to_string()),
103+
None,
104+
None,
105+
)
106+
})?;
107+
let col_count = stmt.column_count();
108+
let col_names: Vec<String> = (0..col_count)
109+
.map(|i| stmt.column_name(i).unwrap_or("?").to_string())
110+
.collect();
111+
let mut rows = Vec::new();
112+
let row_iter = stmt
113+
.query_map([], |row| {
114+
let mut map = IndexMap::new();
115+
for i in 0..col_count {
116+
let name = col_names[i].clone();
117+
let val: String =
118+
row.get::<_, String>(i).unwrap_or_default();
119+
map.insert(name, RuntimeValue::Str(val));
120+
}
121+
Ok(map)
122+
})
123+
.map_err(|e| {
124+
RuntimeError::new(
125+
RuntimeErrorKind::InvalidOperation(e.to_string()),
126+
None,
127+
None,
128+
)
129+
})?;
130+
for row in row_iter {
131+
if let Ok(map) = row {
132+
rows.push(RuntimeValue::Map {
133+
entries: Rc::new(RefCell::new(map)),
134+
is_const: false,
135+
});
136+
}
94137
}
95-
Ok(map)
96-
})
97-
.map_err(|e| {
98-
RuntimeError::new(
99-
RuntimeErrorKind::InvalidOperation(e.to_string()),
138+
Ok(rows)
139+
} else {
140+
Err(RuntimeError::new(
141+
RuntimeErrorKind::InvalidOperation(
142+
"Invalid connection handle".to_string(),
143+
),
100144
None,
101145
None,
102-
)
103-
})?;
104-
for row in row_iter {
105-
if let Ok(map) = row {
106-
rows.push(RuntimeValue::Map {
107-
entries: Rc::new(RefCell::new(map)),
108-
is_const: false,
109-
});
146+
))
110147
}
111-
}
148+
})?;
149+
112150
Ok(RuntimeValue::List {
113151
items: Rc::new(RefCell::new(rows)),
114152
is_const: false,
@@ -123,16 +161,14 @@ impl StdlibRegistry {
123161
name: "close".to_string(),
124162
arity: 1,
125163
callback: |_ctx, args| {
126-
let ptr = args[0].try_into_int().map_err(|_| {
164+
let id = args[0].try_into_int().map_err(|_| {
127165
RuntimeError::new(
128166
RuntimeErrorKind::InvalidOperation("expected int handle".to_string()),
129167
None,
130168
None,
131169
)
132170
})?;
133-
unsafe {
134-
drop(Box::from_raw(ptr as *mut rusqlite::Connection));
135-
}
171+
CONNECTIONS.with(|m| m.borrow_mut().remove(&id));
136172
Ok(RuntimeValue::Null)
137173
},
138174
}),

0 commit comments

Comments
 (0)