-
Notifications
You must be signed in to change notification settings - Fork 169
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
Stuck in infinite loop #205
Comments
I worry the patch does not fix the integer overflow. The code still contains:
All the variables are unsigned int. If want and need are UINT_MAX and bug->used is 0, then it will loop indefinitely. Basically if want is UINT_MAX, then the while condition will be true all the time. But checking want for UINT_MAX is not enough. You will get the same loop any time want >= need, provided need overflows at least once. At each overflow need becomes smaller and smaller until it becomes 0. Then the loop will never terminate because need will be costant 0 and not state changes at any subsequent iteration. |
A note regarding correctness of the patchset: It traded a memory corruption for a data corruption. Now if yajl_buf_ensure_available() cannot fulfill the allocation, it returns an error. yajl_buf_ensure_available() is called only from yajl_buf_append() whose job is to append a data into a buffer. Now on yajl_buf_ensure_available() yajl_buf_append() silently discards the request:
Hence an affected application won't crash and won't loop, but it will truncate data. A correct fix should keep propagating the error back to the application. If it is too difficult to fix it (and I guess it is because yajl cascades many calls without any mechanism for reporting errors), the bundled yajl should rather abort the process. |
@ppisar can you provide a failing test case, I believe the error cascading works correctly and results in an error being returned to the application. The error is recorded on the buffer and attempts to use the data in the buffer will return an error. Lines 365 to 368 in 15093d3
|
I see. I overlooked you are checking the failure later. In that case the error seems indeed propagated. I'm sorry for disturbing you.
|
Hello, I saw CVE-2022-24795 in release notes of GHE v3.4.1, then I'm here after searching a little bit about the CVE. I have had the same remark as Best regards, |
In context of fluentd/td-agent, we ran into an issue, where yajl is running into an infinite loop.
In gdb we could find out, that buf->len is 0 at that time, so need is 0, as well.
yajl-ruby/ext/yajl/yajl_buf.c
Line 62 in 7168bd7
The method is called with the variable want=0, so the shift loop will never exit.
yajl-ruby/ext/yajl/yajl_buf.c
Line 64 in 7168bd7
After digging a little bit deeper, we could see, that the malloc functions are completely missing error handling
yajl-ruby/ext/yajl/yajl_alloc.c
Line 44 in 7168bd7
In our case buf->len was 0, so we assume, that the malloc call failed.
The text was updated successfully, but these errors were encountered: