Re: [PATCH v3] [GSOC] ref-filter: fix read invalid union member bug
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 10, 2021, 07:21 UTC
- Message-ID
- <xmqqh7jbqck1.fsf@gitster.g>
- In-Reply-To
- <pull.949.v3.git.1620487572222.gitgitgadget@gmail.com>
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 24 quoted lines
> 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.
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.
Will queue. Thanks.