fix: CsrfToken::validate_format checking only JWT header (#52)

WalletConnect · Oct 19, 2023 · ec2ec56 · ec2ec56
1 parent a6258b6
commit ec2ec56
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 12 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/integration/integration.test.ts b/integration/integration.test.ts
@@ -160,7 +160,7 @@ describe('verify', () => {
     })
 
     it('doesn\'t allow invalid `token` parameters', async () => {
-      let resp = await http.get(`${url}/index.js?token=<img src onerror=alert(document.domain)>`)
+      let resp = await http.get(`${url}/index.js?token=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.<img src onerror=alert(document.domain)>`)
       expect(resp.status).toBe(400)
     })
   })

diff --git a/src/http_server/attestation.rs b/src/http_server/attestation.rs
@@ -47,7 +47,7 @@ pub(super) async fn post(
     headers: HeaderMap,
     body: Json<Body>,
 ) -> Result<impl IntoResponse, StatusCode> {
-    s.validate_csrf_token(&headers)?;
+    s.token_manager.validate_csrf_token(&headers)?;
 
     s.bouncer
         .set_attestation(&body.attestation_id, &body.origin)

diff --git a/src/http_server/mod.rs b/src/http_server/mod.rs
@@ -33,10 +33,23 @@ mod metrics;
 
 struct Server<B> {
     bouncer: B,
+    token_manager: TokenManager,
+}
+
+struct TokenManager {
     encoding_key: jsonwebtoken::EncodingKey,
     decoding_key: jsonwebtoken::DecodingKey,
 }
 
+impl TokenManager {
+    fn new(secret: &[u8]) -> Self {
+        Self {
+            encoding_key: jsonwebtoken::EncodingKey::from_secret(secret),
+            decoding_key: jsonwebtoken::DecodingKey::from_secret(secret),
+        }
+    }
+}
+
 type State<B> = axum::extract::State<Arc<Server<B>>>;
 
 pub async fn run(
@@ -66,8 +79,7 @@ pub async fn run(
 
     let state = Server {
         bouncer: app,
-        encoding_key: jsonwebtoken::EncodingKey::from_secret(secret),
-        decoding_key: jsonwebtoken::DecodingKey::from_secret(secret),
+        token_manager: TokenManager::new(secret),
     };
 
     let server: Router = Router::new()
@@ -125,7 +137,7 @@ async fn root(
     Ok(match s.bouncer.get_verify_status(project_id).await? {
         VerifyStatus::Disabled => String::new().into_response(),
         VerifyStatus::Enabled { verified_domains } => {
-            let token = s.generate_csrf_token()?;
+            let token = s.token_manager.generate_csrf_token()?;
             let html = index_html(&token);
             let csp = build_content_security_header(verified_domains);
             let headers = [
@@ -163,11 +175,12 @@ impl CsrfToken {
     /// Validates the format of the token without checking either signature or
     /// claims.
     fn validate_format(s: &str) -> bool {
-        jsonwebtoken::decode_header(s).is_ok()
+        s.chars()
+            .all(|c| c.is_ascii_alphanumeric() | matches!(c, '.' | '-' | '_'))
     }
 }
 
-impl<B> Server<B> {
+impl TokenManager {
     fn generate_csrf_token(&self) -> Result<String, Response> {
         use jsonwebtoken::{encode, get_current_timestamp, Header};
 
@@ -233,3 +246,18 @@ fn test_build_content_security_header() {
                          http://*.localhost http://localhost",
     );
 }
+
+#[test]
+fn generated_csrf_tokens_are_valid() {
+    let tm = TokenManager::new(&[]);
+    let token = tm.generate_csrf_token().unwrap();
+    assert!(CsrfToken::validate_format(&token))
+}
+
+#[test]
+fn csrf_validation_checks_jwt_header_and_payload() {
+    let valid_header_invalid_payload =
+        "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.<svg/onload=alert(document.domain)>";
+
+    assert!(!CsrfToken::validate_format(&valid_header_invalid_payload))
+}
diff --git a/src/main.rs b/src/main.rs
@@ -18,10 +18,7 @@ use {
     tap::TapFallible,
     tokio::signal::unix::{signal, SignalKind},
     tracing::info,
-    wc::geoip::{
-        block::{middleware::GeoBlockLayer, BlockingPolicy},
-        MaxMindResolver,
-    },
+    wc::geoip::MaxMindResolver,
 };
 
 #[derive(Deserialize, Debug, Clone)]