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

Weird Commits #845

Closed
sbvoxel opened this issue Apr 26, 2024 · 3 comments
Closed

Weird Commits #845

sbvoxel opened this issue Apr 26, 2024 · 3 comments

Comments

@sbvoxel
Copy link
Contributor

sbvoxel commented Apr 26, 2024

The last two commits are odd.

66e9dff
7e4d5da

The first commit adds a security policy directly copied from a different project without changing the wording (project name), or contact details. Furthermore, is this a good security policy to have? It's the same security policy state actors have inserted into projects to give them time to respond to their intentionally inserted vulnerabilities.

The second commit adds an early return when valuestring is NULL but doesn't clear the object's existing valuestring. It also makes the comment above that branch wrong or incomplete. This is rushed.

@sbvoxel
Copy link
Contributor Author

sbvoxel commented Apr 26, 2024

Though on the second commit, I suppose by returning NULL it means failure, and so in that sense it shouldn't clear the existing valuestring?

@Alanscut
Copy link
Collaborator

Alanscut commented Apr 28, 2024

Hi @sbvoxel
Thank you for your advice.
For the first commit, I didn't check the details, while the contact details are correct. Sorry for my mistake.
The second commit comes from #840 and I agree with you. I believe more similar checks should be made but I didn't have enough time to handle all this then. So I just merged the PR instead of made a release. I will look deep into this today.

It's the same security policy state actors have inserted into projects to give them time to respond to their intentionally inserted vulnerabilities.

Currently I and @PeterAlfredLee are maintaining cjson now. The recent 2 CVE of cjson didn't inform me before they become public - which leave me little time to look into them with details. That's why I pushed a security policy.
IMHO, we need some time to fix security and make a new release before a security issue become public. I don't have a better idea rather then leave a email here. If you have a better idea, feel free to let me know. :)

Alanscut added a commit to Alanscut/cJSON that referenced this issue Apr 28, 2024
Alanscut added a commit that referenced this issue Apr 28, 2024
@sbvoxel
Copy link
Contributor Author

sbvoxel commented Apr 28, 2024

Alright, sounds very good @Alanscut.

Thank you for maintaining cJSON along with the other fellow!

@sbvoxel sbvoxel closed this as completed Apr 28, 2024
Alanscut added a commit to Alanscut/cJSON that referenced this issue Apr 28, 2024
Fix NULL valuestring problem in cJSON_SetValuestring.
This fixes DaveGamble#839 and CVE-2024-31755
Related issue DaveGamble#845
Alanscut added a commit to Alanscut/cJSON that referenced this issue Apr 28, 2024
Fix NULL valuestring problem in cJSON_SetValuestring.
This fixes DaveGamble#839 and CVE-2024-31755
Related issue DaveGamble#845
Alanscut added a commit to Alanscut/cJSON that referenced this issue Apr 28, 2024
Fix NULL valuestring problem in cJSON_SetValuestring.
This fixes DaveGamble#839 and CVE-2024-31755
Related issue DaveGamble#845
Alanscut added a commit to Alanscut/cJSON that referenced this issue Apr 28, 2024
Fix NULL valuestring problem in cJSON_SetValuestring.
This fixes DaveGamble#839 and CVE-2024-31755
Related issue DaveGamble#845
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

No branches or pull requests

2 participants