[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