[PATCH 04/12] passdb: Avoid use-after-free when setting a plaintext password
Kamen Mazdrashki
kamenim at samba.org
Sun Sep 7 20:27:15 MDT 2014
Reviewed-by: Kamen Mazdrashki <kamenim at samba.org>
wow, this code has been there for 4 years and we haven't found this so far!
How amazing that is :)
I would suggest we run regularly (weekly perhaps) a full test suite with
talloc memory squash
(which is now TALLOC_FREE_FILL as far as i can see) + ALWAYS_REALLOC.
It is quite slow I know, but used to run this years ago on a regular basis
and it used to find
some very subtle bugs as far as I can remember.
On Mon, Sep 8, 2014 at 1:30 AM, <abartlet at samba.org> wrote:
> From: Andrew Bartlett <abartlet at samba.org>
>
> The issue here is that pdb_set_plaintext_passwd() re-used the memory from
> pdb_get_pw_history() as input
>
> We need to free this after we copy and set it.
>
> Found by AddressSanitizer
>
> Andrew Bartlett
>
> Change-Id: I4e148e23ccbbe5444c969ff8f91709791c7696bb
> Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> ---
> source3/passdb/pdb_get_set.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c
> index a9b22bb..0d7f4cb 100644
> --- a/source3/passdb/pdb_get_set.c
> +++ b/source3/passdb/pdb_get_set.c
> @@ -873,9 +873,10 @@ bool pdb_set_lanman_passwd(struct samu *sampass,
> const uint8 pwd[LM_HASH_LEN], e
> bool pdb_set_pw_history(struct samu *sampass, const uint8 *pwd, uint32_t
> historyLen, enum pdb_value_state flag)
> {
> if (historyLen && pwd){
> - data_blob_free(&(sampass->nt_pw_his));
> + DATA_BLOB *old_nt_pw_his = &(sampass->nt_pw_his);
> sampass->nt_pw_his = data_blob_talloc(sampass,
> - pwd,
> historyLen*PW_HISTORY_ENTRY_LEN);
> + pwd,
> historyLen*PW_HISTORY_ENTRY_LEN);
> + data_blob_free(old_nt_pw_his);
> if (!sampass->nt_pw_his.length) {
> DEBUG(0, ("pdb_set_pw_history: data_blob_talloc()
> failed!\n"));
> return False;
> --
> 2.1.0
>
>
More information about the samba-technical
mailing list