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

Fix crash found with go-fuzz #131

Merged
merged 1 commit into from Nov 18, 2019
Merged

Conversation

ryarnyah
Copy link
Contributor

When i was testing go-fuzz, i found some crash on pdfcpu.parseObjectStream with this code:

//+build: go-fuzz
package pdfcpu

import (
        "bytes"
        pdf "github.com/pdfcpu/pdfcpu/pkg/pdfcpu"
)

func Fuzz(data []byte) int {
    if _, err := pdf.Read(bytes.NewReader(data), pdf.NewDefaultConfiguration()); err != nil {
      return 0
    }
    return 1
}

You can reproduce it with:

package main

import (
        "os"
        pdf "github.com/pdfcpu/pdfcpu/pkg/pdfcpu" 
)

func main() {
        f, _ := os.Open("crash-18673daced3f9579540897da71f8c9437ae87f44")
        defer f.Close()

        pdf.Read(f, pdf.NewDefaultConfiguration())
}

crash-18673daced3f9579540897da71f8c9437ae87f44.zip

@CLAassistant
Copy link

CLAassistant commented Nov 15, 2019

CLA assistant check
All committers have signed the CLA.

@hhrutter
Copy link
Collaborator

Hi!
Thanks for putting in the time for fuzzing pdfcpu.
Great idea 💚

Unfortunately it is a little bit more complicated.
If you look at the logs below, you'll see an invalid filter FlaseDecode.
I am assuming that go-fuzz fuzzed FlateDecodeto FlaseDecode- which is fine.

The real issue here is handling invalid/unsupported filters.
I have taken the optimistic route until now - logging it and then moving on with fingers crossed 🤞
I think I have to revisit this strategy.

<Filter, FlaseDecode>
	<First, 822>
	<Length, 2233>
	<N, 100>
	<Type, ObjStm>
>>
TRACE: 2019/11/16 22:18:54 decodeStream: decoding filter:FlaseDecode
 INFO: 2019/11/16 22:18:54 Filter not supported: <FlaseDecode>
 READ: 2019/11/16 22:18:54 saveDecodedStreamContent: end
 READ: 2019/11/16 22:18:54 decodeObjectStreams: object stream #1418
 READ: 2019/11/16 22:18:54 decodeObjectStreams: decoding object stream 1418:
 READ: 2019/11/16 22:18:54 parseObjectStream begin: decoding 100 objects.
Fatal: unexpected panic attack: runtime error: slice bounds out of range [:822] with capacity 0
github.com/pdfcpu/pdfcpu/pkg/cli.Process.func1
	/Users/horstrutter/go/src/github.com/pdfcpu/pdfcpu/pkg/cli/process.go:85
runtime.gopanic
	/Users/horstrutter/gotip/src/runtime/panic.go:679
runtime.goPanicSliceAcap
	/Users/horstrutter/gotip/src/runtime/panic.go:93
github.com/pdfcpu/pdfcpu/pkg/pdfcpu.parseObjectStream
	/Users/horstrutter/go/src/github.com/pdfcpu/pdfcpu/pkg/pdfcpu/read.go:346
github.com/pdfcpu/pdfcpu/pkg/pdfcpu.decodeObjectStreams

@ryarnyah
Copy link
Contributor Author

Do you prefer that i close this PR and open an issue instead?

Copy link
Collaborator

@hhrutter hhrutter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix you are suggesting is a side effect of improper handling of unknown filters.
You can fix the root problem in pkg/filter/filter.go in
func NewFilter(filterName string, parms map[string]int) (filter Filter, err error):

...
        case DCT:
		fallthrough
	case JBIG2:
		fallthrough
	case JPX:
		log.Info.Printf("Unsupported filter: <%s>\n", filterName)
		err = ErrUnsupportedFilter

	default:
		err = errors.Errorf("Invalid filter: <%s>\n", filterName)
	}
	return filter, err
}

If you provide a signed commit I can merge this in.

@ryarnyah
Copy link
Contributor Author

I will made these changes tonight, thanks for the proper fix.

@ryarnyah ryarnyah force-pushed the fix/go-fuzz-read branch 7 times, most recently from 8d93ef7 to 6e5759c Compare November 18, 2019 20:40
@ryarnyah
Copy link
Contributor Author

Done with some tests to cover modifications on NewFilter.

Signed-off-by: Ryar Nyah <ryarnyah@gmail.com>
@hhrutter hhrutter merged commit 9f4f093 into pdfcpu:master Nov 18, 2019
@ryarnyah ryarnyah deleted the fix/go-fuzz-read branch November 18, 2019 21:08
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

Successfully merging this pull request may close these issues.

None yet

3 participants