fix: prevent sitekey abuse with account secret authentication for access token validation

SUMMARY
    At present, sitekey can be abused by installing it on a third-party
    site as verifying the access token returned from CAPTCHA validation
    doesn't require any authentication.

    This fix uses account secret authentication to verify access tokens

credits: by @gusted
This commit is contained in:
realaravinth 2022-07-22 19:44:35 +05:30
parent 85f91cb79b
commit 7d0e4c6be4
No known key found for this signature in database
GPG key ID: AD9F0F08E855ED88
4 changed files with 87 additions and 23 deletions

View file

@ -134,6 +134,9 @@ pub trait MCDatabase: std::marker::Send + std::marker::Sync + CloneSPDatabase {
/// get a user's secret
async fn get_secret(&self, username: &str) -> DBResult<Secret>;
/// get a user's secret from a captcha key
async fn get_secret_from_captcha(&self, key: &str) -> DBResult<Secret>;
/// update a user's secret
async fn update_secret(&self, username: &str, secret: &str) -> DBResult<()>;

View file

@ -176,6 +176,10 @@ pub async fn database_works<'a, T: MCDatabase>(
assert!(db.captcha_exists(None, c.key).await.unwrap());
assert!(db.captcha_exists(Some(p.username), c.key).await.unwrap());
// get secret from captcha key
let secret_from_captcha = db.get_secret_from_captcha(&c.key).await.unwrap();
assert_eq!(secret_from_captcha.secret, p.secret, "user secret matches");
// get captcha configuration
let captcha = db.get_captcha_config(p.username, c.key).await.unwrap();
assert_eq!(captcha.key, c.key);

View file

@ -299,6 +299,22 @@ impl MCDatabase for Database {
Ok(secret)
}
/// get a user's secret from a captcha key
async fn get_secret_from_captcha(&self, key: &str) -> DBResult<Secret> {
let secret = sqlx::query_as!(
Secret,
r#"SELECT secret FROM mcaptcha_users WHERE ID = (
SELECT user_id FROM mcaptcha_config WHERE key = $1
)"#,
key,
)
.fetch_one(&self.pool)
.await
.map_err(|e| map_row_not_found_err(e, DBError::AccountNotFound))?;
Ok(secret)
}
/// update a user's secret
async fn update_secret(&self, username: &str, secret: &str) -> DBResult<()> {
sqlx::query!(

View file

@ -29,23 +29,41 @@ pub struct CaptchaValidateResp {
pub valid: bool,
}
#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct VerifyCaptchaResultPayload {
pub secret: String,
pub key: String,
pub token: String,
}
impl From<VerifyCaptchaResultPayload> for VerifyCaptchaResult {
fn from(m: VerifyCaptchaResultPayload) -> Self {
VerifyCaptchaResult {
token: m.token,
key: m.key,
}
}
}
// API keys are mcaptcha actor names
/// route hander that validates a PoW solution token
#[my_codegen::post(path = "V1_API_ROUTES.pow.validate_captcha_token()")]
pub async fn validate_captcha_token(
payload: web::Json<VerifyCaptchaResult>,
payload: web::Json<VerifyCaptchaResultPayload>,
data: AppData,
) -> ServiceResult<impl Responder> {
let secret = data.db.get_secret_from_captcha(&payload.key).await?;
if secret.secret != payload.secret {
return Err(ServiceError::WrongPassword);
}
let payload: VerifyCaptchaResult = payload.into_inner().into();
let key = payload.key.clone();
let res = data
.captcha
.validate_verification_tokens(payload.into_inner())
.await?;
let payload = CaptchaValidateResp { valid: res };
let res = data.captcha.validate_verification_tokens(payload).await?;
let resp = CaptchaValidateResp { valid: res };
data.stats.record_confirm(&data, &key).await?;
//println!("{:?}", &payload);
Ok(HttpResponse::Ok().json(payload))
Ok(HttpResponse::Ok().json(resp))
}
#[cfg(test)]
@ -76,8 +94,21 @@ pub mod tests {
delete_user(data, NAME).await;
register_and_signin(data, NAME, EMAIL, PASSWORD).await;
let (_, _signin_resp, token_key) = add_levels_util(data, NAME, PASSWORD).await;
let (_, signin_resp, token_key) = add_levels_util(data, NAME, PASSWORD).await;
let app = get_app!(data).await;
let cookies = get_cookie!(signin_resp);
let secret = test::call_service(
&app,
test::TestRequest::get()
.cookie(cookies.clone())
.uri(V1_API_ROUTES.account.get_secret)
.to_request(),
)
.await;
assert_eq!(secret.status(), StatusCode::OK);
let secret: db_core::Secret = test::read_body_json(secret).await;
let secret = secret.secret;
let get_config_payload = GetConfigPayload {
key: token_key.key.clone(),
@ -116,11 +147,35 @@ pub mod tests {
assert_eq!(pow_verify_resp.status(), StatusCode::OK);
let client_token: ValidationToken = test::read_body_json(pow_verify_resp).await;
let validate_payload = VerifyCaptchaResult {
let mut validate_payload = VerifyCaptchaResultPayload {
token: client_token.token.clone(),
key: token_key.key.clone(),
secret: NAME.to_string(),
};
// siteverify authentication failure
bad_post_req_test(
data,
NAME,
PASSWORD,
VERIFY_TOKEN_URL,
&validate_payload,
ServiceError::WrongPassword,
)
.await;
// let validate_client_token = test::call_service(
// &app,
// post_request!(&validate_payload, VERIFY_TOKEN_URL).to_request(),
// )
// .await;
// assert_eq!(validate_client_token.status(), StatusCode::OK);
// let resp: CaptchaValidateResp =
// test::read_body_json(validate_client_token).await;
// assert!(resp.valid);
// verifying work
validate_payload.secret = secret.clone();
let validate_client_token = test::call_service(
&app,
post_request!(&validate_payload, VERIFY_TOKEN_URL).to_request(),
@ -139,19 +194,5 @@ pub mod tests {
.await;
let resp: CaptchaValidateResp = test::read_body_json(string_not_found).await;
assert!(!resp.valid);
let validate_payload = VerifyCaptchaResult {
token: client_token.token.clone(),
key: client_token.token.clone(),
};
// key not found
let key_not_found = test::call_service(
&app,
post_request!(&validate_payload, VERIFY_TOKEN_URL).to_request(),
)
.await;
let resp: CaptchaValidateResp = test::read_body_json(key_not_found).await;
assert!(!resp.valid);
}
}