Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds Record For facade/ignition RCE: CVE-2021-3129 #536

Merged
merged 4 commits into from
Mar 3, 2021
Merged

Adds Record For facade/ignition RCE: CVE-2021-3129 #536

merged 4 commits into from
Mar 3, 2021

Conversation

freshleafmedia
Copy link
Contributor

@freshleafmedia freshleafmedia commented Feb 15, 2021

@naderman
Copy link
Contributor

Am I missing something? If I look at https://github.com/facade/ignition/tree/1.16.14 it doesn't seem to actually contain the fix? Did something go wrong with tagging there @freekmurze ?

@freshleafmedia
Copy link
Contributor Author

@naderman
Copy link
Contributor

@freshleafmedia It is, but it looks to me like it's actually a version of 2.5 containing the fix that was accidentally taged as 1.16? The rest of the code is 2.5 now in the 1.16 tag too? https://github.com/facade/ignition/blob/1.16.14/CHANGELOG.md

@naderman
Copy link
Contributor

Regarding your invalid bound change, it was fine to add v1 with the constraint, you just have to then add a lower bound for master at >=2.0-dev because otherwise the two branches overlap?

@freshleafmedia
Copy link
Contributor Author

Really? I was getting an Invalid branch name "v1". error

@naderman
Copy link
Contributor

That's a poorly worded error message I think. It's not the name of the branch that's invalid, it's an invalid branch definition with the name "v1":

Screenshot_20210215_111809

@freshleafmedia
Copy link
Contributor Author

I'm afraid not, It's two separate error messages. If I add a lower version constraint it shows the error:

                                                                                                                        
 [ERROR] Found 1 issue in 1 file.                                                                                       
                                                                                                                        

+------------------------------------+---------------------------+
| File                               | Issues                    |
+------------------------------------+---------------------------+
| facade/ignition/CVE-2021-3129.yaml | Invalid branch name "v1". |
+------------------------------------+---------------------------+

@freshleafmedia
Copy link
Contributor Author

Looking at the validator it is using the regex ^([\d\.\-]+(\.x)?(\-dev)?|master)$ so only a version number, a version number suffixed by -dev or just master on its own are valid options

@naderman
Copy link
Contributor

@freshleafmedia ah sorry, the name is indeed invalid, I suppose "1.x" would be an appropriate branch name to store the info on the 1.x version range, maybe @stof can shed some light on this?

@stof
Copy link
Member

stof commented Feb 15, 2021

I think 1.x is the way to go indeed (assuming that there is indeed a valid fixed version for 1.x)

@valentijnscholten
Copy link

How far is this from getting merged? There seems to be active probing for this vulnerability according to greynoise:
image
https://twitter.com/nathanqthai/status/1367130234663940110

@naderman
Copy link
Contributor

naderman commented Mar 3, 2021

I see that the 1.16.14 release has been fixed in the meantime, so merging it now.

@naderman naderman merged commit 5a6ad8e into FriendsOfPHP:master Mar 3, 2021
@freshleafmedia freshleafmedia deleted the vuln/cve-2021-3129 branch March 4, 2021 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants