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

Encryptor doesn't work with UTF-8 characters #1490

Closed
1 task done
VakarisZ opened this issue Sep 27, 2021 · 1 comment · Fixed by #1508
Closed
1 task done

Encryptor doesn't work with UTF-8 characters #1490

VakarisZ opened this issue Sep 27, 2021 · 1 comment · Fixed by #1508
Labels
Bug An error, flaw, misbehavior or failure in the Monkey or Monkey Island. Complexity: Medium Impact: High

Comments

@VakarisZ
Copy link
Contributor

VakarisZ commented Sep 27, 2021

Describe the bug

Datastore encryptor breaks when a UTF-8 character is encountered:
image

This is an important bug, because gathered credentials might contain unicode characters, user can't submit common passwords, for e.g. "slaptažodis" means password in Lithuanian and might be common in Lithuanian networks:
image

Tasks

  • We should write unit tests that reproduce the issue and then swap out the padding functions with functions from Crypto.Util.Padding. (0d) - @shreyamalviya
@VakarisZ VakarisZ added Bug An error, flaw, misbehavior or failure in the Monkey or Monkey Island. Impact: Critical labels Sep 27, 2021
@mssalvatore
Copy link
Collaborator

mssalvatore commented Sep 27, 2021

This is caused by incorrect assumptions in the padding function.

# TODO: Review and evaluate the security of the padding function
def _pad(self, message):
return message + (self._BLOCK_SIZE - (len(message) % self._BLOCK_SIZE)) * chr(
self._BLOCK_SIZE - (len(message) % self._BLOCK_SIZE)
)
def _unpad(self, message: str):
return message[0 : -ord(message[len(message) - 1])]

The padding function uses len(message), but this assumes that all characters are 1 byte. Unicode characters can be more than 1 byte.

image

This is probably the solution:
image

We should write unit tests that reproduce the issue and then swap out the padding functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug An error, flaw, misbehavior or failure in the Monkey or Monkey Island. Complexity: Medium Impact: High
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants