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 the 'hash' field after 'key' in IDX:branch block #97

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions ubireader/ubifs/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ def __init__(self, buf):
setattr(self, key, fields[key])

idxs = UBIFS_IDX_NODE_SZ
brs = UBIFS_BRANCH_SZ
# Dynamic detection of `brs` (branch size), whether a hash is present after
# the key in authenticated UBIFS. Not checking the len of the hash.
brs = int((len(buf) - UBIFS_IDX_NODE_SZ) / self.child_cnt)
Copy link
Contributor

Choose a reason for hiding this comment

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

The int casting is hiding low-level details. Do you want to round up or round down ? It would be cleaner with math.floor or math.ceil, depending on implementation details. You can also use // instead of / if you want to round down.

Copy link
Author

Choose a reason for hiding this comment

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

I think // is what I was trying to achieve. In theory, if the UBIFS image is correctly formatted, the division by self.child_cnt has no remainder. My int( ... ) was a clumsy attempt to get rid of the .0 in the result of the division. (Because brs needs to be an int, not a float.)

I'll use // instead. I'll also rewrite the comment, because I'm having a hard time to understand myself after a few months.

setattr(self, 'branches', [branch(buf[idxs+(brs*i):idxs+(brs*i)+brs]) for i in range(0, self.child_cnt)])
setattr(self, 'errors', [])

Expand All @@ -183,13 +185,16 @@ class branch(object):
Bin:buf -- Raw data to extract header information from.
"""
def __init__(self, buf):
fields = dict(list(zip(UBIFS_BRANCH_FIELDS, struct.unpack(UBIFS_BRANCH_FORMAT, buf))))
fields = dict(list(zip(UBIFS_BRANCH_FIELDS, struct.unpack(UBIFS_BRANCH_FORMAT, buf[0:UBIFS_BRANCH_SZ]))))
for key in fields:
if key == 'key':
setattr(self, key, parse_key(fields[key]))
else:
setattr(self, key, fields[key])

if len(buf) > (UBIFS_BRANCH_SZ):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is UBIFS_BRANCH_SZ between parentheses ?

Copy link
Author

Choose a reason for hiding this comment

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

For no reason. I forgot to remove them. (Maybe in a previous iteration, this was (UBIFS_BRANCH_SZ + something) ?

I'll remove those unneeded parentheses.

setattr(self, 'hash', buf[UBIFS_BRANCH_SZ:])

setattr(self, 'errors', [])

def __repr__(self):
Expand Down