Bug 8088 - rpccli_samr_chng_pswd_auth_crap segfaults if any input blobs are null
Summary: rpccli_samr_chng_pswd_auth_crap segfaults if any input blobs are null
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.5
Classification: Unclassified
Component: DCE-RPCs and pipes (show other bugs)
Version: 3.5.8
Hardware: All All
: P5 normal
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-14 17:31 UTC by p.mayers
Modified: 2011-08-31 20:56 UTC (History)
1 user (show)

See Also:


Attachments
git-am fix for 3.5.next (1.86 KB, patch)
2011-04-18 21:27 UTC, Jeremy Allison
vl: review+
Details
Patch (1.70 KB, patch)
2011-04-19 17:19 UTC, Volker Lendecke
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description p.mayers 2011-04-14 17:31:01 UTC
rpccli_samr_chng_pswd_auth_crap does an unconditional memcpy:

memcpy(&new_nt_password.data, new_nt_password_blob.data, 516);
memcpy(&new_lm_password.data, new_lm_password_blob.data, 516);
memcpy(&old_nt_hash_enc.hash, old_nt_hash_enc_blob.data, 16);
memcpy(&old_lm_hash_enc.hash, old_lm_hash_enc_blob.data, 16);

...upon entry. However, some combination of the blobs may be NULL, for example new_lm_password_blob or old_lm_hash_enc_blob, if you are processing an MS-CHAPv2 password change.


It ought (I believe?) be possible to call the underlying rpccli_samr_ChangePasswordUser2 with null LM passwords?

You can reproduce this easily with "ntlm_auth --helper-protocol=ntlm-change-password-1" and giving the input:

username: x
nt-domain: y
new-nt-password-blob:: <516 bytes, encoded as 1032 hex, encoded again as base64>
old-nt-hash-blob:: <16 bytes, encoded as 32 hex, encoded again as base64>
.

...and watch the winbind child process crash.
Comment 1 Jeremy Allison 2011-04-18 21:27:23 UTC
Created attachment 6413 [details]
git-am fix for 3.5.next

Can you test this patch please to see if it fixes the problem ?

Thanks,

Jeremy.
Comment 2 Volker Lendecke 2011-04-19 17:19:44 UTC
Created attachment 6416 [details]
Patch

Jeremy, this ports the checks 1:1 to master. Please review & push if you like it.

Thanks,

Volker
Comment 3 Jeremy Allison 2011-04-19 17:28:59 UTC
I already pushed a similar for to master and v3-6-test, thanks !
Jeremy.
Comment 4 Karolin Seeger 2011-04-19 19:18:27 UTC
Pushed to v3-5-test.
Closing out bug report.

Thanks!