All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: ZheNing Hu <adlternative@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com>,
	Git List <git@vger.kernel.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v3] [GSOC] ref-filter: fix read invalid union member bug
Date: Mon, 10 May 2021 20:35:01 +0800	[thread overview]
Message-ID: <CAOLTT8T3yqbM8Ucey6Jfs2U75AP33B4d8W_Ab7ZYDQQDZX-4CQ@mail.gmail.com> (raw)
In-Reply-To: <xmqqh7jbqck1.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> 于2021年5月10日周一 下午3:21写道:
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > used_atom.u is an union, and it has different members depending on
> > what atom the auxiliary data the union part of the "struct
> > used_atom" wants to record. At most only one of the members can be
> > valid at any one time. Since the code checks u.remote_ref without
> > even making sure if the atom is "push" or "push:" (which are only
> > two cases that u.remote_ref.push becomes valid), but u.remote_ref
> > shares the same storage for other members of the union, the check
> > was reading from an invalid member, which was the bug.
> >
> > Modify the condition here to check whether the atom name
> > equals to "push" or starts with "push:", to avoid reading the
> > value of invalid member of the union.
> >
> > Helped-by: Junio C Hamano <gitster@pobox.com>
> > Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> > ---
> >     [GSOC] ref-filter: fix read invalid union member bug
> >
> >     Change from last version:
> >     Modify the processing method of the condition: check whether the name of
> >     the atom equals to "push" or starts with "pushs", which can enhanced
> >     security, although it may bring string match overhead.
>
> I do not think this would have much security implication either
> way.  What it buys us is the future-proofing.
>

Ah, truely.

> I think it is OK to make this change without the enum thing to have
> it graduate early as a fix to the existing code.  The enum thing can
> come on top.
>

Indeed. "enum atom_type" is for ref-filter performance optimization and get
some other benefits like quick index. So I put it in another topic.

> Will queue.  Thanks.

Thanks.
--
ZheNing Hu

  reply	other threads:[~2021-05-10 12:52 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-05 15:31 [PATCH] [GSOC] ref-filter: solve bugs caused by enumeration ZheNing Hu via GitGitGadget
2021-05-06  1:53 ` Junio C Hamano
2021-05-06  5:02   ` ZheNing Hu
2021-05-06  5:35     ` Junio C Hamano
2021-05-06 10:39       ` ZheNing Hu
2021-05-06 11:20         ` Junio C Hamano
2021-05-06 11:52           ` ZheNing Hu
2021-05-06 21:20             ` Junio C Hamano
2021-05-07  4:32               ` ZheNing Hu
2021-05-07  4:49                 ` Junio C Hamano
2021-05-07  5:09                   ` ZheNing Hu
2021-05-06 16:31 ` [PATCH v2] [GSOC] ref-filter: fix read invalid union member bug ZheNing Hu via GitGitGadget
2021-05-08 15:26   ` [PATCH v3] " ZheNing Hu via GitGitGadget
2021-05-10  7:21     ` Junio C Hamano
2021-05-10 12:35       ` ZheNing Hu [this message]
2021-05-10  7:27     ` Junio C Hamano
2021-05-10 12:51       ` ZheNing Hu
2021-05-10 15:01     ` [PATCH v4] " ZheNing Hu via GitGitGadget
2021-05-11  2:29       ` Junio C Hamano
2021-05-11  6:28         ` ZheNing Hu
2021-05-11  9:30           ` Junio C Hamano
2021-05-11 11:47             ` ZheNing Hu
2021-05-11 13:12               ` Junio C Hamano
2021-05-11 13:31                 ` ZheNing Hu
2021-05-11 15:35       ` [PATCH v5] " ZheNing Hu via GitGitGadget
2021-05-12  1:36         ` Junio C Hamano
2021-05-12 10:37           ` ZheNing Hu
2021-05-12 12:12         ` [PATCH v6] " ZheNing Hu via GitGitGadget
2021-05-12 23:24           ` Junio C Hamano
2021-05-13  9:29             ` ZheNing Hu
2021-05-13 15:13           ` [PATCH v7] " ZheNing Hu via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOLTT8T3yqbM8Ucey6Jfs2U75AP33B4d8W_Ab7ZYDQQDZX-4CQ@mail.gmail.com \
    --to=adlternative@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.