From a740f3921714bc2e4f4745be5809202f6adca482 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Sun, 5 Mar 2017 10:30:53 -0500 Subject: [PATCH] canonicalize crate name before checking if it's reserved Currently reserved crate names come from a text file in the source. There are only two entries which consider canonicalization (e.g. the list contains `compiler-rt` and `compiler_rt`). The fact that the name is not canonicalized allows crates like https://crates.io/crates/std to be uploaded. While this definitely isn't a vulnerability, since the registry does *not* canonicalize the name, we should still disallow crates to be uploaded which appear at the same path as a reserved name, and would prevent a crate with a reserved name from being uploaded later. I've moved the actual data out of the text file, and into the database, so we can have a constraint which verifies this at the closest possible location. (We can't use an actual check constraint, as that disallows subselects). The crate referenced above should be manually deleted. --- src/bin/delete-crate.rs | 1 - src/bin/migrate.rs | 37 +++++++++++++++++++++++++++++++++++++ src/krate.rs | 9 +++++---- src/reserved_crates.txt | 40 ---------------------------------------- src/tests/krate.rs | 17 +++++++++++++++++ 5 files changed, 59 insertions(+), 45 deletions(-) delete mode 100644 src/reserved_crates.txt diff --git a/src/bin/delete-crate.rs b/src/bin/delete-crate.rs index 639f0628024..1793b288c3d 100644 --- a/src/bin/delete-crate.rs +++ b/src/bin/delete-crate.rs @@ -7,7 +7,6 @@ #![deny(warnings)] -#[macro_use] extern crate cargo_registry; extern crate postgres; extern crate time; diff --git a/src/bin/migrate.rs b/src/bin/migrate.rs index f2c934aa580..fa905a7d7cf 100644 --- a/src/bin/migrate.rs +++ b/src/bin/migrate.rs @@ -831,6 +831,43 @@ fn migrations() -> Vec { tx.execute("DROP INDEX badges_crate_type", &[])?; Ok(()) }), + Migration::new(20170305095748, |tx| { + tx.batch_execute(" + CREATE TABLE reserved_crate_names ( + name TEXT PRIMARY KEY + ); + CREATE UNIQUE INDEX ON reserved_crate_names (canon_crate_name(name)); + INSERT INTO reserved_crate_names (name) VALUES + ('alloc'), ('arena'), ('ast'), ('builtins'), ('collections'), + ('compiler-builtins'), ('compiler-rt'), ('compiletest'), ('core'), ('coretest'), + ('debug'), ('driver'), ('flate'), ('fmt_macros'), ('grammar'), ('graphviz'), + ('macro'), ('macros'), ('proc_macro'), ('rbml'), ('rust-installer'), ('rustbook'), + ('rustc'), ('rustc_back'), ('rustc_borrowck'), ('rustc_driver'), ('rustc_llvm'), + ('rustc_resolve'), ('rustc_trans'), ('rustc_typeck'), ('rustdoc'), ('rustllvm'), + ('rustuv'), ('serialize'), ('std'), ('syntax'), ('test'), ('unicode'); + + CREATE FUNCTION ensure_crate_name_not_reserved() RETURNS trigger AS $$ + BEGIN + IF canon_crate_name(NEW.name) IN ( + SELECT canon_crate_name(name) FROM reserved_crate_names + ) THEN + RAISE EXCEPTION 'cannot upload crate with reserved name'; + END IF; + RETURN NEW; + END; + $$ LANGUAGE plpgsql; + + CREATE TRIGGER trigger_ensure_crate_name_not_reserved + BEFORE INSERT OR UPDATE ON crates + FOR EACH ROW EXECUTE PROCEDURE ensure_crate_name_not_reserved(); + ")?; + Ok(()) + }, |tx| { + tx.execute("DROP TRIGGER trigger_ensure_crate_name_not_reserved ON crates", &[])?; + tx.execute("DROP FUNCTION ensure_crate_name_not_reserved()", &[])?; + tx.execute("DROP TABLE reserved_crate_names", &[])?; + Ok(()) + }), ]; // NOTE: Generate a new id via `date +"%Y%m%d%H%M%S"` diff --git a/src/krate.rs b/src/krate.rs index b2057221c52..6a228471f6e 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -152,10 +152,11 @@ impl Crate { None => {} } - // Blacklist the current set of crates in the rust distribution - const RESERVED: &'static str = include_str!("reserved_crates.txt"); - - if RESERVED.lines().any(|krate| name == krate) { + let stmt = conn.prepare("SELECT 1 FROM reserved_crate_names + WHERE canon_crate_name(name) = + canon_crate_name($1)")?; + let rows = stmt.query(&[&name])?; + if !rows.is_empty() { return Err(human("cannot upload a crate with a reserved name")) } diff --git a/src/reserved_crates.txt b/src/reserved_crates.txt deleted file mode 100644 index 8d103695940..00000000000 --- a/src/reserved_crates.txt +++ /dev/null @@ -1,40 +0,0 @@ -alloc -arena -ast -builtins -collections -compiler-builtins -compiler-rt -compiler_builtins -compiler_rt -compiletest -core -coretest -debug -driver -flate -fmt_macros -grammar -graphviz -macro -macros -proc_macro -rbml -rust-installer -rustbook -rustc -rustc_back -rustc_borrowck -rustc_driver -rustc_llvm -rustc_resolve -rustc_trans -rustc_typeck -rustdoc -rustllvm -rustuv -serialize -std -syntax -test -unicode diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 8b60d8b77ef..1c8cc90eb6a 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -350,6 +350,23 @@ fn new_krate() { assert_eq!(json.krate.max_version, "1.0.0"); } +#[test] +fn new_krate_with_reserved_name() { + fn test_bad_name(name: &str) { + let (_b, app, middle) = ::app(); + let mut req = ::new_req(app, name, "1.0.0"); + ::mock_user(&mut req, ::user("foo")); + let json = bad_resp!(middle.call(&mut req)); + assert!(json.errors[0].detail.contains("cannot upload a crate with a reserved name")); + } + + test_bad_name("std"); + test_bad_name("STD"); + test_bad_name("compiler-rt"); + test_bad_name("compiler_rt"); + test_bad_name("coMpiLer_Rt"); +} + #[test] fn new_krate_weird_version() { let (_b, app, middle) = ::app();