-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix NTLMv2 hash when username contains non-ASCII characters #56
Conversation
- Also add specs
I noticed there is still an encoding issue using some specific characters (e.g I added a fix and I wrote some specs to show the issue (added to ...
describe '#serialize' do
context 'when the username contains non-ASCI characters' do
let(:t3) {
t2 = Net::NTLM::Message::Type2.new
t2.response(
{
:user => 'Hélène',
:password => '123456',
:domain => ''
},
{
:ntlmv2 => true,
:workstation => 'testlab.local'
}
)
}
it 'serializes without error' do
expect { t3.serialize }.not_to raise_error
end
end
end
... Before the fix
After the fix
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see inline
@@ -39,7 +39,7 @@ def self.decode_utf16le(str) | |||
# the function will convert the string bytes to UTF-16LE and note the encoding as UTF-8 so that byte | |||
# concatination works seamlessly. | |||
def self.encode_utf16le(str) | |||
str.dup.force_encoding('UTF-8').encode(Encoding::UTF_16LE, Encoding::UTF_8).force_encoding('UTF-8') | |||
str.dup.force_encoding('UTF-8').encode(Encoding::UTF_16LE, Encoding::UTF_8).force_encoding('ASCII-8BIT') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this change makes sense and behaves more like the comment block describes, since its supposed to be safe to perform byte concatenation on the result
thanks! |
This fixes #55.
See the issue for details. After the fix: