Issue 206 add input fields character limit

[martgil]

This PR adds a character limit on input fields, including the secure compose message box

close https://github.com/FlowCrypt/flowcrypt-security/issues/206

---

Tests (delete all except exactly one):

• Tests added or updated

---

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

• addresses the issue it closes (if any)
• code is readable and understandable
• is accompanied with tests, or tests are not needed
• is free of vulnerabilities
• is documented clearly and usefully, or doesn't need documentation

[martgil]

Good afternoon, sir, @rrrooommmaaa. I'm trying to add an input limit on the secure compose message box and it doesn't work as expected;

What happens is that onpaste seems not working and doesn't prevent the user from pasting text/contents even though the keypress works just fine.

I tried to try the whole concept at https://jsfiddle.net/ftye9xrn/ starting on the minimal code. The output is much more acceptable there. Do you have any suggestions here?

Furthermore, the goal of this PR is to prevent user's from populating input fields with large texts that cause an unresponsive browser tab (not the entire browser) - related PR https://github.com/FlowCrypt/flowcrypt-security/issues/206. If by any chance you have any feedback or suggestions, please let me know and I'll be happy to share them with Tom.

Thank you!

[rrrooommmaaa]

I tried to try the whole concept at https://jsfiddle.net/ftye9xrn/ starting on the minimal code. The output is much more acceptable there. Do you have any suggestions here?

Hi @martgil. Sorry for the late answer, I somehow missed the notification.
The paste event handler obviously takes the value of the input before the modification, this is why there is no reaction.
The handler for paste event should look somthing like this:

$("#test").on("paste", function (e) {
  const selectedLength = window.getSelection().toString().length;
  const curLength = $("#test").text().length;
  const clipboardLength = e.originalEvent.clipboardData.getData('text').length;
  const resultingLength = curLength - selectedLength + clipboardLength;
  if (resultingLength > 10) {
  	e.preventDefault();
  }
})

However the Selection is a complex object, consisting of possibly more than one range, maybe we need to do something more sophisticated than toString().length on it, I'm encouraging you to further investigate this, @martgil

[rrrooommmaaa]

Probably it's better to use these properties:

• input.selectionStart – position of selection start (writeable),
• input.selectionEnd – position of selection end (writeable),
  as described here

[martgil]

thank you for providing such detailed feedback, sir. I will get back to this after i accomplished some tasks with higher priority. thanks again.

[martgil]

@rrrooommmaaa do you think it's reasonable to move the tasks for adding a length constraint to compose window (contenteditable) to a separate issue so non-complex changes will prioritize to get merged?
Also, should we add a length constraint to the armored key inputs (I can add it in this PR)?

[rrrooommmaaa]

@rrrooommmaaa do you think it's reasonable to move the tasks for adding a length constraint to compose window (contenteditable) to a separate issue so non-complex changes will prioritize to get merged?

Sure, feel free to create a new issue for this sub-task. Please don't forget to include/reference my instructions there.

Also, should we add a length constraint to the armored key inputs (I can add it in this PR)?

I'm not sure about what kind of vulnerability we're trying to protect against here. If we're supposed to accept only a single key there, then it makes sense to limit the length. If multiple keys are allowed, well... depends on performance, I guess. Unless you know of a particular attack.

[gitguardian]

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	Secret	Commit	Filename
4313719	Google OAuth2 Keys	e1a0fe6	extension/js/common/api/email-provider/gmail/google-auth.ts	View secret

🛠 Guidelines to remediate hardcoded secrets

1. Understand the implications of revoking this secret by investigating where it is used in your code.
2. Replace and store your secret safely. Learn here the best practices.
3. Revoke and rotate this secret.
4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

• following these best practices for managing and storing secrets including API keys and other credentials
• install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

---

<sup>🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!</sup>

[martgil]

Sure, feel free to create a new issue for this sub-task. Please don't forget to include/reference my instructions there.

Thanks!

I'm not sure about what kind of vulnerability we're trying to protect against here.

We are trying to mitigate web browser freezing & browser crash when populating input fields with extremely long strings (client-side) - reference: https://github.com/FlowCrypt/flowcrypt-security/issues/206.

To reproduce the issue, download and paste the payload content to any input fields of the browser extension and the browser may start to freeze/crash.

payload.txt

If we're supposed to accept only a single key there, then it makes sense to limit the length. If multiple keys are allowed, well... depends on performance, I guess. Unless you know of a particular attack.

Sorry, I'm just referring with the text length rather than the key length.