fix: forbid stores to the __debug__ builtin #3203

According to [the docs][1], `__debug__` is treated as a constant by the
interpreter.

This patch adds some checks to bring RustPython's behaviour in line with
cpython's.

[1]: https://docs.python.org/3/library/constants.html#built-in-constants
This commit is contained in:
Alex Muscar 2021-10-03 11:46:21 +01:00
parent d0c436949e
commit be92bc5243

View file

@ -22,6 +22,7 @@ use std::borrow::Cow;
type CompileResult<T> = Result<T, CompileError>; type CompileResult<T> = Result<T, CompileError>;
#[derive(PartialEq, Eq, Clone, Copy)]
enum NameUsage { enum NameUsage {
Load, Load,
Store, Store,
@ -51,6 +52,13 @@ impl CallType {
} }
} }
fn is_forbidden_name(name: &str) -> bool {
// See https://docs.python.org/3/library/constants.html#built-in-constants
const BUILTIN_CONSTANTS: &[&str] = &["__debug__"];
BUILTIN_CONSTANTS.contains(&name)
}
/// Main structure holding the state of compilation. /// Main structure holding the state of compilation.
struct Compiler { struct Compiler {
code_stack: Vec<CodeInfo>, code_stack: Vec<CodeInfo>,
@ -375,26 +383,39 @@ impl Compiler {
Ok(()) Ok(())
} }
fn load_name(&mut self, name: &str) { fn load_name(&mut self, name: &str) -> CompileResult<()> {
self.compile_name(name, NameUsage::Load) self.compile_name(name, NameUsage::Load)
} }
fn store_name(&mut self, name: &str) -> CompileResult<()> { fn store_name(&mut self, name: &str) -> CompileResult<()> {
if name.eq("__debug__") { self.compile_name(name, NameUsage::Store)
return Err(self.error(CompileErrorType::SyntaxError(
"cannot assign to __debug__".to_owned(),
)));
}
self.compile_name(name, NameUsage::Store);
Ok(())
} }
fn mangle<'a>(&self, name: &'a str) -> Cow<'a, str> { fn mangle<'a>(&self, name: &'a str) -> Cow<'a, str> {
symboltable::mangle_name(self.class_name.as_deref(), name) symboltable::mangle_name(self.class_name.as_deref(), name)
} }
fn compile_name(&mut self, name: &str, usage: NameUsage) { fn check_forbidden_name(&self, name: &str, usage: NameUsage) -> CompileResult<()> {
if usage == NameUsage::Store && is_forbidden_name(name) {
return Err(self.error(CompileErrorType::SyntaxError(format!(
"cannot assign to {}",
name
))));
}
if usage == NameUsage::Delete && is_forbidden_name(name) {
return Err(self.error(CompileErrorType::SyntaxError(format!(
"cannot delete {}",
name
))));
}
Ok(())
}
fn compile_name(&mut self, name: &str, usage: NameUsage) -> CompileResult<()> {
let name = self.mangle(name); let name = self.mangle(name);
self.check_forbidden_name(&name, usage)?;
let symbol_table = self.symbol_table_stack.last().unwrap(); let symbol_table = self.symbol_table_stack.last().unwrap();
let symbol = symbol_table.lookup(name.as_ref()).expect( let symbol = symbol_table.lookup(name.as_ref()).expect(
"The symbol must be present in the symbol table, even when it is undefined in python.", "The symbol must be present in the symbol table, even when it is undefined in python.",
@ -461,6 +482,8 @@ impl Compiler {
}, },
}; };
self.emit(op(idx as u32)); self.emit(op(idx as u32));
Ok(())
} }
fn compile_statement(&mut self, statement: &ast::Stmt) -> CompileResult<()> { fn compile_statement(&mut self, statement: &ast::Stmt) -> CompileResult<()> {
@ -493,9 +516,9 @@ impl Compiler {
let idx = self.name(part); let idx = self.name(part);
self.emit(Instruction::LoadAttr { idx }); self.emit(Instruction::LoadAttr { idx });
} }
self.store_name(alias)?; self.store_name(alias)?
} else { } else {
self.store_name(name.name.split('.').next().unwrap())?; self.store_name(name.name.split('.').next().unwrap())?
} }
} }
} }
@ -551,9 +574,9 @@ impl Compiler {
// Store module under proper name: // Store module under proper name:
if let Some(alias) = &name.asname { if let Some(alias) = &name.asname {
self.store_name(alias)?; self.store_name(alias)?
} else { } else {
self.store_name(&name.name)?; self.store_name(&name.name)?
} }
} }
@ -772,10 +795,9 @@ impl Compiler {
fn compile_delete(&mut self, expression: &ast::Expr) -> CompileResult<()> { fn compile_delete(&mut self, expression: &ast::Expr) -> CompileResult<()> {
match &expression.node { match &expression.node {
ast::ExprKind::Name { id, .. } => { ast::ExprKind::Name { id, .. } => self.compile_name(id, NameUsage::Delete)?,
self.compile_name(id, NameUsage::Delete);
}
ast::ExprKind::Attribute { value, attr, .. } => { ast::ExprKind::Attribute { value, attr, .. } => {
self.check_forbidden_name(attr, NameUsage::Delete)?;
self.compile_expression(value)?; self.compile_expression(value)?;
let idx = self.name(attr); let idx = self.name(attr);
self.emit(Instruction::DeleteAttr { idx }); self.emit(Instruction::DeleteAttr { idx });
@ -852,6 +874,12 @@ impl Compiler {
.chain(&args.args) .chain(&args.args)
.chain(&args.kwonlyargs); .chain(&args.kwonlyargs);
for name in args_iter { for name in args_iter {
if Compiler::is_forbidden_arg_name(&name.node.arg) {
return Err(self.error(CompileErrorType::SyntaxError(format!(
"cannot assign to {}",
&name.node.arg
))));
}
self.varname(&name.node.arg); self.varname(&name.node.arg);
} }
@ -935,7 +963,7 @@ impl Compiler {
// We have a match, store in name (except x as y) // We have a match, store in name (except x as y)
if let Some(alias) = name { if let Some(alias) = name {
self.store_name(alias)?; self.store_name(alias)?
} else { } else {
// Drop exception from top of stack: // Drop exception from top of stack:
self.emit(Instruction::Pop); self.emit(Instruction::Pop);
@ -992,6 +1020,10 @@ impl Compiler {
Ok(()) Ok(())
} }
fn is_forbidden_arg_name(name: &str) -> bool {
is_forbidden_name(name)
}
fn compile_function_def( fn compile_function_def(
&mut self, &mut self,
name: &str, name: &str,
@ -1107,9 +1139,7 @@ impl Compiler {
self.apply_decorators(decorator_list); self.apply_decorators(decorator_list);
self.store_name(name)?; self.store_name(name)
Ok(())
} }
fn build_closure(&mut self, code: &CodeObject) -> bool { fn build_closure(&mut self, code: &CodeObject) -> bool {
@ -1268,8 +1298,7 @@ impl Compiler {
self.apply_decorators(decorator_list); self.apply_decorators(decorator_list);
self.store_name(name)?; self.store_name(name)
Ok(())
} }
fn load_docstring(&mut self, doc_str: Option<String>) { fn load_docstring(&mut self, doc_str: Option<String>) {
@ -1553,15 +1582,14 @@ impl Compiler {
fn compile_store(&mut self, target: &ast::Expr) -> CompileResult<()> { fn compile_store(&mut self, target: &ast::Expr) -> CompileResult<()> {
match &target.node { match &target.node {
ast::ExprKind::Name { id, .. } => { ast::ExprKind::Name { id, .. } => self.store_name(id)?,
self.store_name(id)?;
}
ast::ExprKind::Subscript { value, slice, .. } => { ast::ExprKind::Subscript { value, slice, .. } => {
self.compile_expression(value)?; self.compile_expression(value)?;
self.compile_expression(slice)?; self.compile_expression(slice)?;
self.emit(Instruction::StoreSubscript); self.emit(Instruction::StoreSubscript);
} }
ast::ExprKind::Attribute { value, attr, .. } => { ast::ExprKind::Attribute { value, attr, .. } => {
self.check_forbidden_name(attr, NameUsage::Store)?;
self.compile_expression(value)?; self.compile_expression(value)?;
let idx = self.name(attr); let idx = self.name(attr);
self.emit(Instruction::StoreAttr { idx }); self.emit(Instruction::StoreAttr { idx });
@ -1950,9 +1978,7 @@ impl Compiler {
conversion: compile_conversion_flag(*conversion), conversion: compile_conversion_flag(*conversion),
}); });
} }
Name { id, .. } => { Name { id, .. } => self.load_name(id)?,
self.load_name(id);
}
Lambda { args, body } => { Lambda { args, body } => {
let prev_ctx = self.ctx; let prev_ctx = self.ctx;
self.ctx = CompileContext { self.ctx = CompileContext {
@ -2153,6 +2179,12 @@ impl Compiler {
let (size, unpack) = self.gather_elements(additional_positional, args)?; let (size, unpack) = self.gather_elements(additional_positional, args)?;
let has_double_star = keywords.iter().any(|k| k.node.arg.is_none()); let has_double_star = keywords.iter().any(|k| k.node.arg.is_none());
for keyword in keywords {
if let Some(name) = &keyword.node.arg {
self.check_forbidden_name(name, NameUsage::Store)?;
}
}
let call = if unpack || has_double_star { let call = if unpack || has_double_star {
// Create a tuple with positional args: // Create a tuple with positional args:
self.emit(Instruction::BuildTuple { size, unpack }); self.emit(Instruction::BuildTuple { size, unpack });