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

A question about the location of the root cause #21

Open
HotSpurzzZ opened this issue Aug 2, 2023 · 0 comments
Open

A question about the location of the root cause #21

HotSpurzzZ opened this issue Aug 2, 2023 · 0 comments

Comments

@HotSpurzzZ
Copy link

First of all thank you very much for your work, I am reproducing the tool, but I have a problem with the location of the root cause, for example in the target libjpeg_cve-2017-15232, the program crashes because of 'output_buf' in jquant1.c:536 is an illegal address, so the author repaired it by checking whether the 'output_buf' is empty before that. Similarly, RCABeach also uses jquant1.c:521 - 534 as the location of the root cause.

But after analyzing the vulnerability, I think it is necessary to find out what variable causes the 'output_buf' to be empty here, and this variable should also be the root cause of the crash. Of course, it is completely correct to use the author's repair location as the benchmark for root cause positioning, but for root cause analysis tools, its analysis of vulnerabilities may be more in-depth.

For example, in libjpeg_cve-2017-15232, 'output_buf' comes from jdapistd.c:316, where the second variable NULL causes 'output_buf' to be empty. And the reason why line 316 is called is because 'num_lines' of line 315 is not zero.

//jdapistd.c:315-316
  for (n = 0; n < num_lines; n++)
    jpeg_read_scanlines(cinfo, NULL, 1);

And 'num_lines' comes from line jdapistd.c:339 and line jdapistd.c:451.

//jdapistd.c:339
  rows_left = rows % cinfo->max_v_samp_factor;
//jdapistd.c:451
  lines_to_read = lines_after_iMCU_row - lines_to_skip;

As shown in the code below, the variables 'lines_after_iMCU' and 'lines_to_skip' come from other locations, and by comparing with the input that does not cause a crash, the abnormal variable comes from 'lines_per_iMCU', which is defined in line 379

//jdapistd.c:379
  lines_per_iMCU_row = cinfo->_min_DCT_scaled_size * cinfo->max_v_samp_factor;
  ......
  ......
//jdapistd.c:445    In normal execution, the value of lines_per_iMCU_row is 8, and the value of lines_to_skip is 16. In PoC execution, the value of lines_per_iMCU_row is 24, resulting in the value of lines_to_skip being 0
    lines_to_skip = (lines_after_iMCU_row / lines_per_iMCU_row) *
                    lines_per_iMCU_row;

So, can line 379 and the definition of the variable 'cinfo->max_v_samp_factor' be a candidate for the root cause, which is the cause of the initial attempt to affect 'output_buf' to NULL.

Thanks again for your work!

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

1 participant