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

Parse attrib-bits field when its included in file attributes #139

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

bschmeck
Copy link

Version 5 of the SFTP spec introduced an optional attrib-bits field (spec). When a server returns that data and we don't handle the 4 byte field, we quickly wind up parsing gibberish. (In my case, I wound up looping forever over a non-existent ACL entry and OOM'd my box.)

Changes between v4 and v5: https://datatracker.ietf.org/doc/html/draft-ietf-secsh-filexfer-05#section-10.1
v5 file attributes struct definition: https://datatracker.ietf.org/doc/html/draft-ietf-secsh-filexfer-05#section-5

The attrib-bits field was introduced in V05 of the SFTP spec and needs to be
parsed from the attributes struct when parsing file data.  Without handling
attrib-bits, we wind up off by 4 bytes for each entry in the filename data and
quickly wind up parsing gibberish.
@mfazekas
Copy link
Collaborator

@bschmeck thanks looks great to me. Does it makes sense to add tests too?!

@bschmeck
Copy link
Author

bschmeck commented Aug 9, 2022

Sorry for the delay here, I got pulled into other things and then went on vacation.

I realized that version 6 also has an attributes bits field, so I reworked things slightly and now V06::Attributes inherits from V05::Attributes and the F_BITS constant is defined in V05::Attributes.

I also added test for the new V05::Attributes class, those should have been there from the start!

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.

2 participants