Skip to content

Commit 3d2e107

Browse files
committed
Error on invalid token for registry auth
When using registry operations with authentication there will be now an error if the given token is not valid. This is a technically a breaking change because a registry might give some tokens which will be denied by these new checks. In practice these tokens cause issues with HTTP so no registry should generate them.
1 parent 2f84e1a commit 3d2e107

File tree

3 files changed

+54
-0
lines changed

3 files changed

+54
-0
lines changed

crates/crates-io/lib.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@ impl Registry {
394394
Some(s) => s,
395395
None => bail!("no upload token found, please run `cargo login`"),
396396
};
397+
check_token(token)?;
397398
headers.append(&format!("Authorization: {}", token))?;
398399
}
399400
self.handle.http_headers(headers)?;
@@ -510,3 +511,23 @@ pub fn is_url_crates_io(url: &str) -> bool {
510511
.map(|u| u.host_str() == Some("crates.io"))
511512
.unwrap_or(false)
512513
}
514+
515+
/// Checks if a token is valid or malformed.
516+
///
517+
/// This check is necessary to prevent sending tokens which create an invalid HTTP request.
518+
/// It would be easier to check just for alphanumeric tokens, but we can't be sure that all
519+
/// registries only create tokens in that format so that is as less restricted as possible.
520+
pub fn check_token(token: &str) -> Result<()> {
521+
let is_valid = token.bytes().all(|b| {
522+
b >= 32 // undefined in ISO-8859-1, in ASCII/ UTF-8 not-printable character
523+
&& b < 128 // utf-8: the first bit signals a multi-byte character
524+
&& b != 127 // 127 is a control character in ascii and not in ISO 8859-1
525+
|| b == b't' // tab is also allowed (even when < 32)
526+
});
527+
528+
if is_valid {
529+
Ok(())
530+
} else {
531+
Err(anyhow::anyhow!("invalid token."))
532+
}
533+
}

src/cargo/ops/registry.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,7 @@ pub fn registry_login(
901901
if tok.is_empty() {
902902
bail!("please provide a non-empty token");
903903
}
904+
crates_io::check_token(tok.as_ref().expose())?;
904905
}
905906
}
906907
if &reg_cfg == &new_token {

tests/testsuite/login.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,38 @@ fn empty_login_token() {
126126
.run();
127127
}
128128

129+
#[cargo_test]
130+
fn invalid_login_token() {
131+
let registry = RegistryBuilder::new()
132+
.no_configure_registry()
133+
.no_configure_token()
134+
.build();
135+
setup_new_credentials();
136+
137+
let check_ = |stdin: &str, stderr: &str| {
138+
cargo_process("login")
139+
.replace_crates_io(registry.index_url())
140+
.with_stdout("please paste the token found on [..]/me below")
141+
.with_stdin(stdin)
142+
.with_stderr(stderr)
143+
.with_status(101)
144+
.run();
145+
};
146+
let check = |stdin: &str| {
147+
check_(stdin, "[ERROR] invalid token.");
148+
};
149+
// first check updates index so it must be handled differently
150+
check_(
151+
"😄",
152+
"\
153+
[UPDATING] crates.io index
154+
[ERROR] invalid token.",
155+
);
156+
check("\u{0016}");
157+
check("\u{0000}");
158+
check("你好");
159+
}
160+
129161
#[cargo_test]
130162
fn bad_asymmetric_token_args() {
131163
// These cases are kept brief as the implementation is covered by clap, so this is only smoke testing that we have clap configured correctly.

0 commit comments

Comments
 (0)