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

Prevent NoMethodError when @name is nil in entry #382

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zelivans
Copy link
Contributor

Bug found by fuzzing rubyzip.

In the #name_is_directory? method the @name variable is used to check if the entry is a directory. This results in an unhandled NoMethodError when @name is nil. In this case @name was set in #read_c_dir_entry, where I added a quick fix by setting it to an empty string when it would be set to nil.

I do think some more serious refactoring might be needed to prevent bugs like this one from happening with rubyzip but for now this solves this precise crash.

Raw crash

/home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/entry.rb:109:in `name_is_directory?': undefined method `end_with?' for nil:NilClass (NoMethodError)
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/entry.rb:346:in `set_ftype_from_c_dir_entry'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/entry.rb:390:in `read_c_dir_entry'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/entry.rb:205:in `read_c_dir_entry'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/central_directory.rb:127:in `block in read_central_directory_entries'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/central_directory.rb:126:in `times'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/central_directory.rb:126:in `read_central_directory_entries'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/central_directory.rb:138:in `read_from_stream'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/file.rb:76:in `block in initialize'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/file.rb:75:in `open'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/file.rb:75:in `initialize'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/file.rb:97:in `new'
	from /home/ariel/afl-kisaten/private/sandbox/rubyzip/rubyzip/lib/zip/file.rb:97:in `open'
	from zip.rb:3:in `<main>'

Reproduce with this file

@coveralls
Copy link

Coverage Status

Coverage increased (+4.6%) to 100.0% when pulling a2cba27 on zelivans:fix_read_3 into d07b13a on rubyzip:master.

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

2 participants