Skip to content

Commit

Permalink
Only allow http(s) scheme for urls (ref #3505) (#3508)
Browse files Browse the repository at this point in the history
With this change only http(s) schemes are allowed for post.url
field. This is checked for incoming api and federation requests.
Existing posts in database which are sent to clients are not
checked. Neither does it check urls in markdown.
  • Loading branch information
Nutomic authored Jul 6, 2023
1 parent c12feda commit 00f9f79
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 2 deletions.
3 changes: 2 additions & 1 deletion crates/api_crud/src/post/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use lemmy_utils::{
error::LemmyError,
utils::{
slurs::{check_slurs, check_slurs_opt},
validation::{clean_url_params, is_valid_body_field, is_valid_post_title},
validation::{check_url_scheme, clean_url_params, is_valid_body_field, is_valid_post_title},
},
};
use tracing::{warn, Instrument};
Expand All @@ -58,6 +58,7 @@ impl PerformCrud for CreatePost {

is_valid_post_title(&data.name)?;
is_valid_body_field(&data.body, true)?;
check_url_scheme(&data.url)?;

check_community_ban(local_user_view.person.id, data.community_id, context.pool()).await?;
check_community_deleted_or_removed(data.community_id, context.pool()).await?;
Expand Down
3 changes: 2 additions & 1 deletion crates/api_crud/src/post/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use lemmy_utils::{
error::LemmyError,
utils::{
slurs::check_slurs_opt,
validation::{clean_url_params, is_valid_body_field, is_valid_post_title},
validation::{check_url_scheme, clean_url_params, is_valid_body_field, is_valid_post_title},
},
};

Expand Down Expand Up @@ -50,6 +50,7 @@ impl PerformCrud for EditPost {
}

is_valid_body_field(&data.body, true)?;
check_url_scheme(&data.url)?;

let post_id = data.post_id;
let orig_post = Post::read(context.pool(), post_id).await?;
Expand Down
2 changes: 2 additions & 0 deletions crates/apub/src/objects/post.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ use lemmy_utils::{
markdown::markdown_to_html,
slurs::{check_slurs_opt, remove_slurs},
time::convert_datetime,
validation::check_url_scheme,
},
};
use std::ops::Deref;
Expand Down Expand Up @@ -191,6 +192,7 @@ impl Object for ApubPost {
} else {
None
};
check_url_scheme(&url)?;

let local_site = LocalSite::read(context.pool()).await.ok();
let allow_sensitive = local_site_opt_to_sensitive(&local_site);
Expand Down
19 changes: 19 additions & 0 deletions crates/utils/src/utils/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,12 +302,22 @@ pub fn check_site_visibility_valid(
Ok(())
}

pub fn check_url_scheme(url: &Option<Url>) -> LemmyResult<()> {
if let Some(url) = url {
if url.scheme() != "http" && url.scheme() != "https" {
return Err(LemmyError::from_message("invalid_url_scheme"));
}
}
Ok(())
}

#[cfg(test)]
mod tests {
use super::build_totp_2fa;
use crate::utils::validation::{
build_and_check_regex,
check_site_visibility_valid,
check_url_scheme,
clean_url_params,
generate_totp_2fa_secret,
is_valid_actor_name,
Expand Down Expand Up @@ -519,4 +529,13 @@ mod tests {
assert!(check_site_visibility_valid(false, false, &Some(true), &None).is_ok());
assert!(check_site_visibility_valid(false, false, &None, &Some(true)).is_ok());
}

#[test]
fn test_check_url_scheme() {
assert!(check_url_scheme(&None).is_ok());
assert!(check_url_scheme(&Some(Url::parse("http://example.com").unwrap())).is_ok());
assert!(check_url_scheme(&Some(Url::parse("https://example.com").unwrap())).is_ok());
assert!(check_url_scheme(&Some(Url::parse("ftp://example.com").unwrap())).is_err());
assert!(check_url_scheme(&Some(Url::parse("javascript:void").unwrap())).is_err());
}
}

0 comments on commit 00f9f79

Please sign in to comment.