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

Please check if affected by CVE-2020-12762 #161

Closed
Whissi opened this issue May 15, 2020 · 21 comments
Closed

Please check if affected by CVE-2020-12762 #161

Whissi opened this issue May 15, 2020 · 21 comments

Comments

@Whissi
Copy link
Contributor

Whissi commented May 15, 2020

CVE-2020-12762 was reported for json-c, see json-c/json-c#592. Please check if libfastjson is affected by a similar problem. At least it looks like that printbuf.c changes should be backported.

@s-kostyuk
Copy link

Well, looks like libfastjson shares some common and not-yet-patched code with json-c. I slightly modified the original application from json-c/json-c#592 and got a segmentation fault on my machine with libfastjson. Reproduced this issue with on 32-bit and x86_64 builds of the sample application.

Environment:

  • OS: Ubuntu 20.04 x86_64
  • libfastjson version: 0.99.8-2, installed from apt

Sample application:

#include <libfastjson/json_util.h>
#include <unistd.h>

int main() {
  fjson_object_from_fd(STDIN_FILENO);
  return 0;
}

Proof:

user@computer:~$ dd if=/dev/zero of=poc.json bs=1 count=1 seek=2147483647
user@computer:~$ (dd if=poc.json bs=4096; sleep 1; dd if=test.json bs=10) 2>/dev/null |  ~/Projects/build-fastjson-test-Desktop-Debug/fastjson-test 
Segmentation fault (core dumped)

@apple-ouyang
Copy link
Contributor

apple-ouyang commented Mar 10, 2023

Have this CVE been fixed in libfastjson?

@apple-ouyang
Copy link
Contributor

I compare ths json-c fix for this CVE and the libfastjson code.
json-c modify these files:

  • arraylist.c: The chagned function is different from the function in libfastjson, so we don't need to chagne.
  • hashlink.c: libfastjson doesn't use this file, so we don't need to change.
  • printbuf.c: libfastjson and json-c both use the same code, so maybe we can patch as the json-c do?

@rgerhards
Copy link
Member

Thx for the heads-up. While you are at it: it would be great if you could craft a PR.

@apple-ouyang
Copy link
Contributor

Ok~ I will craft a PR this evening later. I wanna repruduce this CVE problem and check whether the json-c patch could fix it. After that I will push the PR.

@apple-ouyang
Copy link
Contributor

Sorry for the late PR #166

I reproduce and solve the CVE by referencing the json-c patch

I think I have solved this CVE.

@apple-ouyang
Copy link
Contributor

@rgerhards Could you please check this CVE fix? I've verified the CVE is fixed.

@xiaoge1001
Copy link

Hello, can anyone review the code?

@rgerhards
Copy link
Member

yes, this fixes it.

1 similar comment
@rgerhards
Copy link
Member

yes, this fixes it.

@xiaoge1001
Copy link

yes, this fixes it.

So who can merge the code?

@rgerhards
Copy link
Member

I usually keep it open until we finally release. But I can merge right now if you prefer.

@rgerhards
Copy link
Member

actually I can't right now due to some operational issues at github. Will do as soon as it is possible again.

@xiaoge1001
Copy link

I hope the fix will be in the code repository as soon as possible. Thanks.
But I've found that it's done.

@apple-ouyang
Copy link
Contributor

@Whissi I think the CVE is fixed, so if there is no other problem, maybe you can close the issue I guess?

@EmielBruijntjes
Copy link
Contributor

Will there also be a new release-tag, like v1.0 or so with this fix?

@Whissi
Copy link
Contributor Author

Whissi commented Mar 29, 2023

There will be a new tag. Remember that rsyslog project is doing releases every ~2 months so maybe this will be addressed already in the upcoming April release. If not it will get tackled in the summer release.

I'll leave this bug open as reminder for the team. Once they do the changelog/release work, they can close it.

@rgerhards
Copy link
Member

I can add a tag if @EmielBruijntjes needs this for an internal build process. Just let me know.

@EmielBruijntjes
Copy link
Contributor

Yes, that would be nice, it makes life on our side a bit easier if we can refer to a version number or a tag.

@rgerhards
Copy link
Member

@EmielBruijntjes I have added the v0.99.9.1 tag.

@rgerhards
Copy link
Member

Side-Note: there is also the upcoming version numbering change in that tag included. Just so that you know.

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

6 participants