Dump file compatibility, read GPT from block device, py3 compatible KILO challenge/response #27
Conversation
tuxuser
changed the title
|
Thank you for this PR. For writing (nothing is changed on the disk): In each case I did a lglaf.py --cr before to initiate a KILO chalenge/response but that seems to only allow commands but nothing changed for partitions. But it's still a realy good step, thank you ! |
dump-file.py
Outdated
| if not output.startswith(path + ' '): | ||
| # Replace multiple whitespaces with single ws | ||
| # Example input: "-rwxr-x--- root root 496888 1970-01-01 00:00 lafd" | ||
| output = re.sub(' +', ' ', output) |
There was a problem hiding this comment.
No need to do this if you use output.split() without arguments. The old code was better I think as it checked explicitly for the path (and would catch errors should the stat binary be missing or something unexpected like that).
Could you mention in the commit message why the old stat approach no longer works?
There was a problem hiding this comment.
On recent devices the stat binary is not allowed to be called, hence the change to "ls". Sure, I will mention that in the commit msg. Would I amend the commit and force push - or how to do it cleanly?
partitions.py
Outdated
| @@ -229,7 +219,7 @@ def wipe_partition(comm, disk_fd, part_offset, part_size): | |||
| help="Write file to partition on device ('-' for stdin)") | |||
| parser.add_argument("--wipe", action='store_true', | |||
| help="TRIMs a partition") | |||
| parser.add_argument("partition", nargs='?', | |||
| parser.add_argument("partition", nargs='?', type=int, | |||
There was a problem hiding this comment.
type=int prevents the name (e.g. 'recovery') from working
partitions.py
Outdated
| parser.error(e) | ||
|
|
||
| info = get_partition_info_string(part) | ||
| _logger.debug(info) |
There was a problem hiding this comment.
Use _logger.debug("%s", info) to avoid crashing if info contains %
partitions.py
Outdated
| if isinstance(query, str): | ||
| result = [p for p in diskinfo.gpt.partitions if p.name == query] | ||
| elif isinstance(query, int): | ||
| result = [p for p in diskinfo.gpt.partitions if p.index == query] |
There was a problem hiding this comment.
Rather than forcing args.partition to be an integer, what about:
def find_partition(diskinfo, query):
partno = int(query) if query.isdigit() else None
for part in diskinfo.gpt.partitions:
if part.index == partno or part.name == query:
return part
raise ValueError("Partition not found: %s" % query)That looks a bit cleaner to me
There was a problem hiding this comment.
I agree, will change that!
|
Requested changes pushed |
dump-file.py
Outdated
| if not len(output): | ||
| _logger.debug("Output: %r", output) | ||
| raise RuntimeError("Cannot get filesize for %s" % path) | ||
| size_bytes = output.split(' ')[3] | ||
| # Example output: "-rwxr-x--- root root 496888 1970-01-01 00:00 lafd" | ||
| size_bytes = output.split()[3] |
There was a problem hiding this comment.
This can still crash if LG somehow decides to change the output format, could you do a stricter check here? FWIW, the output of ls is not consistent everywhere. On Linux, I have this for coreutils 8.28 and busybox 1.27.2:
crw-rw-rw- 1 root root 1, 3 Nov 21 21:15 /dev/null
brw-rw---- 1 root disk 8, 0 Nov 21 21:15 /dev/sda
-rw-r--r-- 1 root root 65355 Jun 10 2013 /etc/mime.types
-r--r--r-- 1 root root 0 Nov 25 20:12 /proc/cpuinfo
On an older CM 13.0 snapshot using toolbox in normal mode, it looks like this:
brw------- 1 root root 179, 0 1970-02-26 14:26 /dev/block/mmcblk0
crw-rw-rw- 1 root root 1, 3 1970-02-26 14:26 /dev/null
-rw-r--r-- 1 root root 56 2009-01-01 08:00 /etc/hosts
-r--r--r-- 1 root root 0 2017-11-25 20:17 /proc/cpuinfo
What about this logic:
...
output = output.decode('utf8')
if path not in output:
raise RuntimeError("Cannot find path %s in ls output" % path)
fields = output.split()
if len(fields) >= 7:
for field in fields[3:]:
if field.isdigit():
return int(field)
_logger.debug("ls output: %s", output)
raise RuntimeError("Cannot find filesize for path %s" % path)
``
Btw, please add the `-d` option (if supported by the `ls` command), otherwise you will list directory contentsThere was a problem hiding this comment.
The path check is problematic:
"ls -ld /sbin/lafd" / "ls -l /sbin/lafd"
returns:
"-rwxr-x--- root root 496888 1970-01-01 00:00 lafd"
So we cannot check for the full path in the output string. I would rather keep the check "if not len(output)" to catch a not-found file
|
Done |
|
Looks good, can you rebase and squash some fixup commits and acknowledge the authors (e.g. those from #19) in the commit message? |
…irmware prevents using "stat", "ls" however is supported on old and new firmware.
…commands, which is restricted on recent devices
Credits: @joeblowma => Initial reverse engineering @snoremaster3000 => Porting the C code to python @steadfasterX => Pushing the code along to this repo
|
Done ;) |
|
this still doesn't succeed in giving a working shell on ght LG G3 D852 - i reported this in #31. |
Both changes target compatibility with more recent devices and are more reliable than parsing shell cmd output.
UPDATE:
Adding py3 compatible KILO challenge/response via --cr switch:
Added mode argument to specify on function call, mode 2 is currently used afaik.
Made the key switchable via field USE_MFG_KEY - Normally we use the production key so there is no need to expose this switch on cmdline.
PS: I did not go for the mentioned struct pack/unpack changes on purpose, it decreases readability imho.
Credits to:
@joeblowma => Initial reverse engineering - most difficult part, easy for joeblowma tho ;)
@snoremaster3000 => Porting the C code to python
@steadfasterX => Pushing the code along to this repo
dump_file: Rather than using "stat -t" for enumerating the size of a file, use "ls -l". After removing the multiple whitespaces aka. tabs it gives the filesize in it's 3rd column.
partitions / extract_partitions: Instead of reading partition info by parsing shell output, GPT is read directly from mmcblk0 block device (LBA length: 34). For parsing the GPT a library from @jrd is used (jrd's Github Repo, also MIT licensed). I removed the call to ioctl for local disk as we are working with a remote device. The library is also utilized for pretty-printing.
Further, the parameter "partition" in partitions.py is set to type "int" to use it more safely, without additional conversion.Fixes issues:
#9 partitions.py --list gives "AssertionError: Expected arrow in ls output"
#24 LG Q6: additional limitations from LG? (Point 1 and 3)
#26 offset partition download
Obsolete Pull Requests:
#12 added challenge response algo
#19 add challenge/response
#22 fix: Expected arrow in ls output