-
Notifications
You must be signed in to change notification settings - Fork 219
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
Telegram iOS DB parser enhancements and bug fixes (#1998, #2000, #2003 and #2005) #1999
Conversation
Thank you @wladimirleite! I'll collect samples to help testing this a bit more. |
Processing 12 Telegram iOS databases I collected here, I got:
|
Hi @wladimirleite! Here are the testing results on 19 Telegram iOS databases I collected here so far:
After commit I pushed, we are recognizing more Telegram databases. In the other hand, 13 new exceptions are being thrown because of |
Great! |
I didn't, this week I'm in a meeting all days... I'll send you by Teams, thank you very much @wladimirleite! |
Just sent. |
@lfcnassif, taking a closer look in the samples you sent me and others I have here, I think that the databases with different set tables, like (t0, t2 and t3 only) and (t15, t16, t17, t19, t20 and t21), should not be identified as "application/x-telegram-db-ios". Although they are named "db_sqlite" and used by Telegram, they contain different types of information, not related to contacts or messages chats. And they are not a newer version of the Telegram database we support. Below is an example of a folder structure present in a very recent iOS extraction. So I am not sure if the changes you made for #2000 are the best approach. Tables actually used are t0, t2, t6, t7 and t9. |
@lfcnassif, I found another (not directly related) issue that some databases I have here are not being parsed because of an exception in contacts parsing. For example, when extracting the "title" property of a contact, it calls that function to find a "t". This is rare, but it happens a couple of times in the DB samples I have. |
Sorry @wladimirleite, I didn't test my change for false positive detections, my fault.
I totally agree with both recommendations, testing just for those tables and removing detection by name, I didn't know other different databases with db_sqlite name exist.
I see... If it is is a heuristic to read serialized data (I didn't check the code) maybe using a proper deserialize function may help... |
As this is not related to the original issues, and this is clearly a bug (although rare), I am creating another issue. |
There are other things that could be improved in the Telegram internal parser, but this PR already include too many changes. |
Thank you very much @wladimirleite! @hauck-jvsh, since you are the original author of this parser, could you review this PR when you have available time? |
I can review, if time isn't a problem, this when I return from my vacation. |
I will start reviewing this tomorrow, is there any chances of getting some IOS databases? @lfcnassif, @wladimirleite |
I am sending you (through Teams) the samples I have (the whole folder, which include also a few Android databases). It also includes iOS samples @lfcnassif sent me recently. |
Thank you @hauck-jvsh! I think all my samples are included in @wladimirleite's data set. |
Excellent work, you have rewrite the PostBoxCoding from scratch, now you are decoding the entire object instead of the previous approach of find specific key inside the bytes. I think that now it is much more general, preventing decoding data with the wrong type or even data from other places. |
@wladimirleite, I'm running some tests here and it is taking a lot of time to process. I think that this is same problem you mentioned before, that messages have thousands of number in the messageTo metadata. Did you forget to push the changes? |
Yes, it takes way too long for some DBs. Discussing with @lfcnassif, I created a new issue (#2011), as it is not directly related to the issues addressed by this PR, and ideally any solution (probably changing the current behavior) would have to be applied to other chat parsers. |
After digging a little I manage to extract some thumbs from the database. |
Just one thing, previously the default behavior was to use the UFED resultfor IOS and the internal parser for Android, but now the default behavior will change, but it looks good to me, I think that we can merge. |
Sure! I will take a look as soon as possible. |
@hauck-jvsh, the thumbnail extraction worked great! I ran a few more tests, checking with an iPhone that I am working on, and everything looks fine. I just pushed two minor commits (code formatting and preserving line breaks in messages). The line breaks make some messages more readable (I made a similar change in WhatsApp parser recently). |
I think that this is ready for merging. I had to reinstall my eclipse and I didn't apply the project formatting style. |
7a0cc1c
to
183016b
Compare
Sorry guys, I had to force push to fix a wrong conflict resolution pushed before, seems fine now. Thank you very much @wladimirleite and @hauck-jvsh for this great work! |
Closes #1998, #2000, #2003 and #2005.