TypeError: Cannot read properties of undefined (reading 'samplesPerLine')

[manunio]

While Fuzzing locally using jazzer.js JpegImage().parse threw TypeError: Cannot read properties of undefined (reading 'samplesPerLine') at

pdf.js/src/core/jpg.js

Line 1076 in d8424a4

this.width = frame.samplesPerLine;

Configuration:

• Operating system and its version: Ubuntu 20.04.6 LTS
• PDF.js version: v4.0.189

Steps to reproduce the problem:

❯ cat test.js
import { JpegImage } from "./src/core/jpg.js";
new JpegImage().parse(Buffer.from('ffd8', 'hex'));

❯ node test.js
Warning: JpegImage.parse - reached the end of the image data without finding an EOI marker (0xFFD9).
file:///home/maxx/dev/security/oss-fuzz-projects/pdf.js/src/core/jpg.js:1076
    this.width = frame.samplesPerLine;
                       ^

TypeError: Cannot read properties of undefined (reading 'samplesPerLine')
    at JpegImage.parse (file:///home/maxx/dev/security/oss-fuzz-projects/pdf.js/src/core/jpg.js:1076:24)
    at file:///home/maxx/dev/security/oss-fuzz-projects/pdf.js/test.js:3:17
    at ModuleJob.run (node:internal/modules/esm/module_job:218:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:329:24)
    at async loadESM (node:internal/process/esm_loader:34:7)
    at async handleMainPromise (node:internal/modules/run_main:113:12)

Node.js v21.1.0

What is the expected behavior?

• Should not throw exception.

[manunio]

Hi @Snuffleupagus would be interested in fuzzing pdf.js at oss-fuzz ? It's a free service by google for continuously fuzzing important open source projects.

[Snuffleupagus]

would be interested in fuzzing pdf.js at oss-fuzz ? It's a free service by google for continuously fuzzing important open source projects.

Thanks for the offer!
For me personally, the utility of that would depend a fair bit on the number and "quality" of issues it'd report.

For example: A lot of the worker-thread code, which is where the parsing happens, will be called with data that's already been validated elsewhere; hence I'd be slightly worried about us potentially "drowning" in unrelated/non-actionable reports.[1]

/cc @calixteman, @marco-c What's your opinion on this?

---

[1] Note that the patch for this issue simply "replaced" an implicit Error with an explicit one, see the discussion starting at #17303 (review).

[pyoor]

@Snuffleupagus can you point me to where in the worker-thread code this validation occurs? Assuming that that code is exported, the fuzzer implementation could pass data through that validation routing before then passing it to the individual parsers (which I definitely think is a worthwhile fuzzing target).

[Snuffleupagus]

can you point me to where in the worker-thread code this validation occurs?

There's no "point" where that happens, since we have lots of validation all over the place (which you need given how real-world PDF documents often look).
My comment was simply intended to illustrate that getting loads of reports similar to this one, which as discussed in #17303 (review) wasn't clear that we actually "must" fix, could end up taking a lot of triaging resources.

[tysmith]

could end up taking a lot of triaging resources.

That is a valid concern but it's better to see what bugs are found and triage as time allows. Invalid issues can always be marked as wontfix.

[manunio]

Thanks for the offer! For me personally, the utility of that would depend a fair bit on the number and "quality" of issues it'd report.

For example: A lot of the worker-thread code, which is where the parsing happens, will be called with data that's already been validated elsewhere; hence I'd be slightly worried about us potentially "drowning" in unrelated/non-actionable reports.[1]

/cc @calixteman, @marco-c What's your opinion on this?

[1] Note that the patch for this issue simply "replaced" an implicit Error with an explicit one, see the discussion starting at #17303 (review).

Hi thanks for your feedback, jazzer.js(libfuzzer) catches such reports fairly early that's how it did for above report, After the fix and seeing it running for sometime i have not seen such issue being raised. The reason for selecting ImageDecoders was, as it being listed as an example and it seems its also being released seperately as pdfjs-dist. In short we can fine tune fuzzing code(the one calling parse in this case) to ignore such errors. Should we start with decoders for now to see how it goes, i can submit a pr as an initial step.

[Snuffleupagus]

The reason for selecting ImageDecoders was, as it being listed as an example and it seems its also being released separately as pdfjs-dist.

Note that when the image decoders are used "normally", i.e. during parsing of a PDF document, any errors thrown during image decoding will be caught and thus cannot break general parsing of a PDF page.

Should we start with decoders for now to see how it goes, i can submit a pr as an initial step.

We should probably wait for feedback from @calixteman and @marco-c first, before doing something that might add a lot of maintenance/triaging work.

[marco-c]

@manunio if you are willing to work on the integration and on an initial triage of the problems found, we could make a try.

You can file a single issue (so that it doesn't pollute the repo too much) with a list of problems found that you think are relevant, and we can triage as time allows.

[manunio]

@manunio if you are willing to work on the integration and on an initial triage of the problems found, we could make a try.

You can file a single issue (so that it doesn't pollute the repo too much) with a list of problems found that you think are relevant, and we can triage as time allows.

Yes, I'm happy to help whenever I can, but to meet oss-fuzz's requirement for project acceptance, they ask for email(s) addresses from your end. This helps ensure that project maintainers get bug reports effectively.

Note the email(s) affiliated with the project will be public in the OSS-Fuzz repo, as they will be part of a configuration file.

[marco-c]

I think before adding ourselves to OSS-Fuzz we should first try a bit manually, otherwise we will be overwhelmed by automatically filed issues.

[manunio]

I think before adding ourselves to OSS-Fuzz we should first try a bit manually, otherwise we will be overwhelmed by automatically filed issues.

That's what I did locally, I keep it running for a few hours to check early crashes, this issue was its initial finding and have not found anything yet. Please note that oss-fuzz will not file github issues unless specifically stated in it's config file but you will receive the mail though.

Another alternative will be ClusterFuzzLite its based on Clusterfuzz which powers oss-fuzz, it runs as a part of CI workflow.

[marco-c]

Let's go ahead then, you can add my email (mcastelluccio@mozilla.com).