mirror of
https://github.com/teloxide/teloxide.git
synced 2024-10-24 09:57:18 +02:00
Hide bot token in errors
This fixes a potential[^1] security vulnerability -- if bot shows errors from teloxide to the user & for some reason network error happened[^2] the url of the request would be included in the error. Since TBA includes bot token in the error this may lead to token leakage. This commit fixes that issue by removing the token from the urls of `reqwest::Error`, we try to only replace the token, but if we fail we remove the whole url. This can be tested by using a very low timeout value for the http reqwest client: ```rust let client = reqwest::Client::builder() .timeout(std::time::Duration::from_millis(1)) .build() .unwrap(); let bot = Bot::from_env_with_client(client).auto_send(); // see if the token is redacted when network error (timeout) happens // while sending common requests let _ = dbg!(bot.get_me().await); // see if the token is redacted when network error (timeout) happens // while downloading files ("path" is unimportant as the timeout is so // low the request probably won't even be sent) let _ = dbg!(bot.download_file_stream("path").next().await); ``` For me this gives the following result: ```text [t.rs:26] bot.get_me().await = Err( Network( reqwest::Error { kind: Request, url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some( Domain( "api.telegram.org", ), ), port: None, path: "/token:redacted/GetMe", query: None, fragment: None, }, source: TimedOut, }, ), ) [t.rs:31] bot.download_file_stream("path").next().await = Some( Err( reqwest::Error { kind: Request, url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some( Domain( "api.telegram.org", ), ), port: None, path: "/file/token:redacted/path", query: None, fragment: None, }, source: TimedOut, }, ), ) ``` Note that this commits parent is `d0be260` and not the current master the master branch currently contains breaking changes (we'll need to make a release from this brach directly). [^1]: Note that there are recorded cases where the token got exposed. [^2]: Note that this can be theoretically be controlled by the user when sending/downloading bigger files.
This commit is contained in:
parent
d0be260575
commit
daec5ee13e
5 changed files with 70 additions and 17 deletions
|
@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
|
||||
## unreleased
|
||||
|
||||
## 0.4.5 - 2022-04-03
|
||||
|
||||
### Fixed
|
||||
|
||||
- Hide bot token in errors ([#200][200])
|
||||
|
||||
[200]: https://github.com/teloxide/teloxide-core/pull/200
|
||||
|
||||
## 0.4.4 - 2022-04-21
|
||||
|
||||
### Added
|
||||
|
|
|
@ -1,7 +1,7 @@
|
|||
[package]
|
||||
name = "teloxide-core"
|
||||
description = "Core part of the `teloxide` library - telegram bot API client"
|
||||
version = "0.4.4"
|
||||
version = "0.4.5"
|
||||
edition = "2018"
|
||||
|
||||
license = "MIT"
|
||||
|
|
|
@ -41,6 +41,7 @@ impl<'w> Download<'w> for Bot {
|
|||
&self.token,
|
||||
path,
|
||||
)
|
||||
.map(|res| res.map_err(crate::errors::hide_token))
|
||||
.boxed()
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1,19 +1,19 @@
|
|||
use std::io;
|
||||
|
||||
use derive_more::From;
|
||||
use serde::Deserialize;
|
||||
use thiserror::Error;
|
||||
|
||||
/// An error caused by downloading a file.
|
||||
#[derive(Debug, Error, From)]
|
||||
#[derive(Debug, Error)]
|
||||
pub enum DownloadError {
|
||||
/// A network error while downloading a file from Telegram.
|
||||
#[error("A network error: {0}")]
|
||||
// NOTE: this variant must not be created by anything except the From impl
|
||||
Network(#[source] reqwest::Error),
|
||||
|
||||
/// An I/O error while writing a file to destination.
|
||||
#[error("An I/O error: {0}")]
|
||||
Io(#[source] std::io::Error),
|
||||
Io(#[from] std::io::Error),
|
||||
}
|
||||
|
||||
/// An error caused by sending a request to Telegram.
|
||||
|
@ -35,6 +35,7 @@ pub enum RequestError {
|
|||
|
||||
/// Network error while sending a request to Telegram.
|
||||
#[error("A network error: {0}")]
|
||||
// NOTE: this variant must not be created by anything except the From impl
|
||||
Network(#[source] reqwest::Error),
|
||||
|
||||
/// Error while parsing a response from Telegram.
|
||||
|
@ -727,3 +728,54 @@ pub enum ApiError {
|
|||
#[error("Unknown error: {0:?}")]
|
||||
Unknown(String),
|
||||
}
|
||||
|
||||
impl From<reqwest::Error> for DownloadError {
|
||||
fn from(error: reqwest::Error) -> Self {
|
||||
DownloadError::Network(hide_token(error))
|
||||
}
|
||||
}
|
||||
|
||||
impl From<reqwest::Error> for RequestError {
|
||||
fn from(error: reqwest::Error) -> Self {
|
||||
RequestError::Network(hide_token(error))
|
||||
}
|
||||
}
|
||||
|
||||
/// Replaces token in the url in the error with `token:redacted` string.
|
||||
pub(crate) fn hide_token(mut error: reqwest::Error) -> reqwest::Error {
|
||||
let url = match error.url_mut() {
|
||||
Some(url) => url,
|
||||
None => return error,
|
||||
};
|
||||
|
||||
if let Some(mut segments) = url.path_segments() {
|
||||
// Usually the url looks like "bot<token>/..." or "file/bot<token>/...".
|
||||
let (beginning, segment) = match segments.next() {
|
||||
Some("file") => ("file/", segments.next()),
|
||||
segment => ("", segment),
|
||||
};
|
||||
|
||||
if let Some(token) = segment.and_then(|s| s.strip_prefix("bot")) {
|
||||
// make sure that what we are about to delete looks like a bot token
|
||||
if let Some((id, secret)) = token.split_once(':') {
|
||||
if secret.len() >= 35
|
||||
&& secret
|
||||
.chars()
|
||||
.all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_')
|
||||
&& id.chars().all(|c| c.is_ascii_digit())
|
||||
{
|
||||
// found token, hide only the token
|
||||
let without_token =
|
||||
&url.path()[(beginning.len() + "/bot".len() + token.len())..];
|
||||
let redacted = format!("{beginning}token:redacted{without_token}");
|
||||
|
||||
url.set_path(&redacted);
|
||||
return error;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// couldn't find token in the url, hide the whole url
|
||||
error.without_url()
|
||||
}
|
||||
|
|
|
@ -37,18 +37,14 @@ where
|
|||
let request = client
|
||||
.post(crate::net::method_url(api_url, token, method_name))
|
||||
.multipart(params)
|
||||
.build()
|
||||
.map_err(RequestError::Network)?;
|
||||
.build()?;
|
||||
|
||||
// FIXME: uncomment this, when reqwest starts setting default timeout early
|
||||
// if let Some(timeout) = timeout_hint {
|
||||
// *request.timeout_mut().get_or_insert(Duration::ZERO) += timeout;
|
||||
// }
|
||||
|
||||
let response = client
|
||||
.execute(request)
|
||||
.await
|
||||
.map_err(RequestError::Network)?;
|
||||
let response = client.execute(request).await?;
|
||||
|
||||
process_response(response).await
|
||||
}
|
||||
|
@ -81,18 +77,14 @@ where
|
|||
.post(crate::net::method_url(api_url, token, method_name))
|
||||
.header(CONTENT_TYPE, HeaderValue::from_static("application/json"))
|
||||
.body(params)
|
||||
.build()
|
||||
.map_err(RequestError::Network)?;
|
||||
.build()?;
|
||||
|
||||
// FIXME: uncomment this, when reqwest starts setting default timeout early
|
||||
// if let Some(timeout) = timeout_hint {
|
||||
// *request.timeout_mut().get_or_insert(Duration::ZERO) += timeout;
|
||||
// }
|
||||
|
||||
let response = client
|
||||
.execute(request)
|
||||
.await
|
||||
.map_err(RequestError::Network)?;
|
||||
let response = client.execute(request).await?;
|
||||
|
||||
process_response(response).await
|
||||
}
|
||||
|
@ -105,7 +97,7 @@ where
|
|||
tokio::time::sleep(DELAY_ON_SERVER_ERROR).await;
|
||||
}
|
||||
|
||||
let text = response.text().await.map_err(RequestError::Network)?;
|
||||
let text = response.text().await?;
|
||||
|
||||
serde_json::from_str::<TelegramResponse<T>>(&text)
|
||||
.map_err(|source| RequestError::InvalidJson {
|
||||
|
|
Loading…
Reference in a new issue