finnbear / rustrict

rustrict is a profanity filter for Rust
https://crates.io/crates/rustrict
MIT License
94 stars 10 forks source link

Don't Mark Accents as Censored #14

Closed shemmings6 closed 1 year ago

shemmings6 commented 1 year ago

Motivation

It is causing valid names to be marked as censored

Summary

While investigating why words with accented characters were marked as censored, I noticed this comment on the censor function

    /// # Unfortunate Side Effects
    ///
    /// All diacritical marks (accents) are removed by the current implementation. This is subject
    /// to change, as a better implementation would make this optional.

i made a test case to see the difference in text once run through censor and what it was marked as

    #[test]
    fn test() {
        let filter = RustrictFilter {
            ignore_false_positives: false,
            ignore_self_censoring: false,
            temp_censor_replacement: '\u{00a0}',
            regex: Regex::new(format!("{}+", '\u{00a0}').as_str()).unwrap(),
            censor_replacement: "đŸ¤«".to_string(),
        };

        let valid_name = "ErnĂ©sto Jose DurĂ¡n Lar";
        println!("{}", valid_name);
        let result = filter.filter(valid_name, Severity::ModerateOrHigher);
        println!("{}", result);
        assert!(!filter.is_censored(valid_name, Severity::ModerateOrHigher));
    }

This outputs a failed test on the assert that the name is not censored:

ErnĂ©sto Jose DurĂ¡n Lar
Ernesto Jose Duran Lar
thread 'filter::test::test' panicked at 'assertion failed: !filter.is_censored(valid_name, Severity::ModerateOrHigher)', libraries/s6-validations/src/filter.rs:218:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
   2: core::panicking::panic
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:117:5
   3: s6_validations::filter::test::test
             at ./src/filter.rs:218:9
   4: s6_validations::filter::test::test::{{closure}}
             at ./src/filter.rs:205:15
   5: core::ops::function::FnOnce::call_once
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/ops/function.rs:250:5
   6: core::ops::function::FnOnce::call_once
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
test filter::test::test ... FAILED

Could a change be made so that at minimum, this is not considered censored text?

Alternatives

Accented characters are not removed at all and they are not marked as censored

Context

I am using rustrict version 0.4.0

finnbear commented 1 year ago

Hello!

I'm not sure what you are doing in RustrictFilter::is_censored but, if I had to guess, you are checking if the uncensored text equals the censored text to determine validity. If true, this is problematic since the intended way to tell if text is "valid" is to check the Type output of Censor::analyze or Censor::censor_and_analyze using Type::is. The String output of Censor::censor or Censor::censor_and_analyze is only intended to be a censored subset of the original input, and differences from the original input do not necessarily imply any particular reason for censoring. As you discovered in the documentation, a limitation of the current algorithm is removing diacritics while censoring. A possible workaround is to use the original text if the Type is appropriate:

let input = String::from("ErnĂ©sto Jose DurĂ¡n Lar");
let (censored, analysis) = Censor::from_str(&input).censor_and_analyze();
let output = if analysis.is(Type::INAPPROPRIATE) {
   // bad word was detected, only pass through the censored version (you could also reject the input and ask the user to try again)
   censored
} else {
   // no bad words detected, allow the input with accents
   input
};

To be clear, there is a tradeoff. The above may allow some malicious inputs that exploit diacritics as part of the profanity.

shemmings6 commented 1 year ago

Yes you're right, turns out we are doing a string comparison.

Are there any plans to not remove accent marks? Instead of returning sanitized input (stripping accents) - could it only return profanity modifications?

We will be moving forward with using the analysis approach for now though, thank you for your quick response!

finnbear commented 1 year ago

Are there any plans to not remove accent marks?

I haven't found a way to preserve accents but, if and when I do, I'll make this change. The challenging part is that accents are removed in a pre-processing step, so the rest of the filter doesn't have to handle them. Either innocent accents need to be added back or not removed in the first place. Not removing accents seems preferable, but would mean the more complex parts of the filter would have to handle them.

shemmings6 commented 1 year ago

the approach i ended up going with to try to avoid some malicious inputs like you mentioned was to make is_censored function like this:

fn is_censored(&self, input: &str, severity: Severity) -> bool {
        !self
            .filter(input, severity)
            .eq(&diacritics::remove_diacritics(input))
    }