Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Strange input results in false positive #49

Closed
3 of 5 tasks
HatScripts opened this issue Jan 7, 2024 · 2 comments
Closed
3 of 5 tasks

bug: Strange input results in false positive #49

HatScripts opened this issue Jan 7, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@HatScripts
Copy link
Contributor

Expected behavior

When I input the following string:

    "" ""
    "" ""
    "" ""
Assamese -> Assam

I expect that there should be no censoring.

Actual behavior

However, Assam becomes A*sam.

Strangely, modifying parts of the string, such as the quotes ("), results in no censoring.

Minimal reproducible example

import {
  RegExpMatcher,
  TextCensor,
  englishDataset,
  englishRecommendedTransformers,
  keepStartCensorStrategy,
  keepEndCensorStrategy,
  asteriskCensorStrategy
} from 'obscenity'

const matcher = new RegExpMatcher({
  ...englishDataset.build(),
  ...englishRecommendedTransformers
})

const strategy = keepStartCensorStrategy(keepEndCensorStrategy(asteriskCensorStrategy()))
const censor = new TextCensor().setStrategy(strategy)

const input = `    "" ""
    "" ""
    "" ""
Assamese -> Assam`

const matches = matcher.getAllMatches(input)
console.log(censor.applyTo(input, matches))

Steps to reproduce

  1. Run the above code
  2. View console

Additional context

No response

Node.js version

N/A

Obscenity version

0.2.0

Priority

  • Low
  • Medium
  • High

Terms

  • I agree to follow the project's Code of Conduct.
  • I have searched existing issues for similar reports.
@HatScripts HatScripts added the bug Something isn't working label Jan 7, 2024
@HatScripts
Copy link
Contributor Author

const input = `    
    
    
Assamese -> Assam`

Additionally, removing the quotes and leaving just the whitespace (4 spaces on each line) still results in the unexpected censoring. If you delete any of these spaces, no censoring occurs.

@jo3-l
Copy link
Owner

jo3-l commented Jan 7, 2024

The error seems to be in the whitelisted term matching logic. In particular, we are using an index into the original input where we should instead be using an index to the transformed input, resulting in the second assa to be skipped over*. The following diff seems to fix it, if this is urgent for you:

diff --git a/src/matcher/regexp/RegExpMatcher.ts b/src/matcher/regexp/RegExpMatcher.ts
index 7f4fdb1..af31d87 100644
--- a/src/matcher/regexp/RegExpMatcher.ts
+++ b/src/matcher/regexp/RegExpMatcher.ts
@@ -161,7 +161,7 @@ export class RegExpMatcher implements Matcher {
                                }
 
                                matches.insert(indices[startIndex], endIndex);
-                               lastEnd = endIndex + 1;
+                               lastEnd = startIndex + whitelistedTerm.length;
                        }
                }

I will hold off on a patch release until I have time to look at this more carefully, though. The matching logic is fairly complex and I would like to refamiliarize myself with the implementation to ensure this is fully correct first (particularly in cases with non-ASCII characters.) Unfortunately, as I said in #46, this may have to wait until late this month or early February. Apologies.

*I verified that there is no security issue with OOB access due to this mismatch--it should be purely a matter of correctness.

jo3-l added a commit that referenced this issue Aug 2, 2024
@jo3-l jo3-l closed this as completed in ebf95ad Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants