bug: normalize id elements to remove potential wrap characters (#748)

* bug: normalize id elements to remove potential wrap characters
* chore: Update vendored SDK to use protobuf 2.16.2

Remove potential "{}" wrapper from bsoids and collection ids. (See original issue for details)

Test included.

Try sending in a sync request with a bso_id wrapped and URLencoded.

Closes #680

Cargo.toml

@@ -62,6 +62,7 @@ slog-stdlog = "4.0"
 slog-term = "2.6"
 time = "0.2"
 url = "2.1"
+urlencoding = "1.1"
 uuid = { version = "0.8.1", features = ["serde", "v4"] }
 validator = "0.10"
 validator_derive = "0.10"

src/error.rs

@@ -76,6 +76,9 @@ pub enum ApiErrorKind {
 
     #[fail(display = "{}", _0)]
     Validation(#[cause] ValidationError),
+
+    #[fail(display = "Invalid Submission: {}", _0)]
+    InvalidSubmission(String),
 }
 
 impl ApiError {
@@ -216,6 +219,7 @@ impl From<Context<ApiErrorKind>> for ApiError {
                 StatusCode::INTERNAL_SERVER_ERROR
             }
             ApiErrorKind::Validation(error) => error.status,
+            ApiErrorKind::InvalidSubmission(_) => StatusCode::BAD_REQUEST,
         };
 
         Self { inner, status }
@@ -269,7 +273,8 @@ impl Serialize for ApiErrorKind {
         match *self {
             ApiErrorKind::Db(ref error) => serialize_string_to_array(serializer, error),
             ApiErrorKind::Hawk(ref error) => serialize_string_to_array(serializer, error),
-            ApiErrorKind::Internal(ref description) => {
+            ApiErrorKind::Internal(ref description)
+            | ApiErrorKind::InvalidSubmission(ref description) => {
                 serialize_string_to_array(serializer, description)
             }
             ApiErrorKind::Validation(ref error) => Serialize::serialize(error, serializer),

src/web/extractors.rs

@@ -28,7 +28,7 @@ use validator::{Validate, ValidationError};
 
 use crate::db::transaction::DbTransactionPool;
 use crate::db::{util::SyncTimestamp, DbPool, Sorting};
-use crate::error::ApiError;
+use crate::error::{ApiError, ApiErrorKind};
 use crate::server::{metrics, ServerState, BSO_ID_REGEX, COLLECTION_ID_REGEX};
 use crate::settings::{Secrets, ServerLimits};
 use crate::web::{
@@ -66,6 +66,22 @@ pub struct UidParam {
     uid: u64,
 }
 
+fn clean_entry(s: &str) -> Result<String, ApiError> {
+    // URL decode and check that the string is all ascii.
+    let decoded: String = match urlencoding::decode(s) {
+        Ok(v) => v,
+        Err(e) => {
+            debug!("unclean entry: {:?} {:?}", s, e);
+            return Err(ApiErrorKind::Internal(e.to_string()).into());
+        }
+    };
+    if !decoded.is_ascii() {
+        debug!("unclean entry, non-ascii value in {:?}", decoded);
+        return Err(ApiErrorKind::InvalidSubmission("invalid value".into()).into());
+    };
+    Ok(decoded)
+}
+
 #[derive(Clone, Debug, Deserialize, Validate)]
 pub struct BatchBsoBody {
     #[validate(custom = "validate_body_bso_id")]
@@ -300,7 +316,20 @@ impl FromRequest for BsoBodies {
                 }
                 // Save all id's we get, check for missing id, or duplicate.
                 let bso_id = if let Some(id) = bso.get("id").and_then(serde_json::Value::as_str) {
-                    let id = id.to_string();
+                    let id = match clean_entry(&id.to_string()) {
+                        Ok(v) => v,
+                        Err(_) => {
+                            return future::err(
+                                ValidationErrorKind::FromDetails(
+                                    "Invalid BSO ID".to_owned(),
+                                    RequestErrorLocation::Body,
+                                    Some("bsos".to_owned()),
+                                    None,
+                                )
+                                .into(),
+                            );
+                        }
+                    };
                     if bso_ids.contains(&id) {
                         return future::err(
                             ValidationErrorKind::FromDetails(
@@ -518,8 +547,17 @@ impl BsoParam {
             ))?;
         }
         if let Some(v) = elements.get(5) {
-            let sv = String::from_str(v).map_err(|_| {
-                warn!("⚠️ Invalid BsoParam Error: {:?}", v; tags);
+            let sv = clean_entry(&String::from_str(v).map_err(|e| {
+                warn!("⚠️ Invalid BsoParam Error: {:?} {:?}", v, e; tags);
+                ValidationErrorKind::FromDetails(
+                    "Invalid BSO".to_owned(),
+                    RequestErrorLocation::Path,
+                    Some("bso".to_owned()),
+                    Some(tags.clone()),
+                )
+            })?)
+            .map_err(|e| {
+                warn!("⚠️ Invalid BsoParam Error: {:?} {:?}", v, e; tags);
                 ValidationErrorKind::FromDetails(
                     "Invalid BSO".to_owned(),
                     RequestErrorLocation::Path,
@@ -585,14 +623,22 @@ impl CollectionParam {
             return Ok(None);
         }
         if let Some(v) = elements.get(4) {
-            let sv = String::from_str(v).map_err(|_e| {
+            let mut sv = String::from_str(v).map_err(|_e| {
                 ValidationErrorKind::FromDetails(
                     "Missing Collection".to_owned(),
                     RequestErrorLocation::Path,
                     Some("collection".to_owned()),
                     Some(tags.clone()),
                 )
             })?;
+            sv = clean_entry(&sv).map_err(|_e| {
+                ValidationErrorKind::FromDetails(
+                    "Invalid Collection".to_owned(),
+                    RequestErrorLocation::Path,
+                    Some("collection".to_owned()),
+                    Some(tags.clone()),
+                )
+            })?;
             Ok(Some(Self { collection: sv }))
         } else {
             Err(ValidationErrorKind::FromDetails(
@@ -1113,7 +1159,20 @@ impl HawkIdentifier {
         // path: "/1.5/{uid}"
         let elements: Vec<&str> = uri.path().split('/').collect();
         if let Some(v) = elements.get(2) {
-            u64::from_str(v).map_err(|e| {
+            let clean = match clean_entry(v) {
+                Err(e) => {
+                    warn!("⚠️ HawkIdentifier Error invalid UID {:?} {:?}", v, e);
+                    return Err(ValidationErrorKind::FromDetails(
+                        "Invalid UID".to_owned(),
+                        RequestErrorLocation::Path,
+                        Some("uid".to_owned()),
+                        tags,
+                    )
+                    .into());
+                }
+                Ok(v) => v,
+            };
+            u64::from_str(&clean).map_err(|e| {
                 warn!("⚠️ HawkIdentifier Error invalid UID {:?} {:?}", v, e);
                 ValidationErrorKind::FromDetails(
                     "Invalid UID".to_owned(),
@@ -1700,7 +1759,9 @@ fn validate_body_bso_sortindex(sort: i32) -> Result<(), ValidationError> {
 
 /// Verifies the BSO id string is valid
 fn validate_body_bso_id(id: &str) -> Result<(), ValidationError> {
-    if !VALID_ID_REGEX.is_match(id) {
+    let clean =
+        clean_entry(id).map_err(|_| request_error("Invalid id", RequestErrorLocation::Body))?;
+    if !VALID_ID_REGEX.is_match(&clean) {
         return Err(request_error("Invalid id", RequestErrorLocation::Body));
     }
     Ok(())
@@ -2138,6 +2199,31 @@ mod tests {
         assert_eq!(&result.collection, "tabs");
     }
 
+    #[test]
+    fn test_quoted_bso() {
+        let payload = HawkPayload::test_default(*USER_ID);
+        let altered_bso = format!("\"{{{}}}\"", *USER_ID);
+        let state = make_state();
+        let uri = format!(
+            "/1.5/{}/storage/tabs/{}",
+            *USER_ID,
+            urlencoding::encode(&altered_bso)
+        );
+        let header = create_valid_hawk_header(&payload, &state, "GET", &uri, TEST_HOST, TEST_PORT);
+        let req = TestRequest::with_uri(&uri)
+            .data(state)
+            .header("authorization", header)
+            .header("accept", "application/json,text/plain:q=0.5")
+            .method(Method::GET)
+            .to_http_request();
+        req.extensions_mut().insert(make_db());
+        let result = block_on(BsoRequest::extract(&req))
+            .expect("Could not get result in test_valid_collection_request");
+        // make sure the altered bsoid matches the unaltered one, without the quotes and cury braces.
+        assert_eq!(result.user_id.legacy_id, *USER_ID);
+        assert_eq!(altered_bso.as_str(), result.bso);
+    }
+
     #[test]
     fn test_invalid_collection_request() {
         let hawk_payload = HawkPayload::test_default(*USER_ID);

vendor/mozilla-rust-sdk/googleapis/src/spanner.rs

@@ -36,7 +36,7 @@ impl Client {
         let creds = ChannelCredentials::google_default_credentials()?;
 
         // Create a Spanner client.
-        let chan = ChannelBuilder::new(env.clone())
+        let chan = ChannelBuilder::new(env)
             .max_send_message_len(100 << 20)
             .max_receive_message_len(100 << 20)
             .secure_connect(&endpoint, creds);