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

Empty text and timeline file generated by Noriben #37

Open
VikramKharvi opened this issue Mar 29, 2020 · 16 comments
Open

Empty text and timeline file generated by Noriben #37

VikramKharvi opened this issue Mar 29, 2020 · 16 comments

Comments

@VikramKharvi
Copy link

The current release doesn't print anything in txt and timeline file. Git committed on Mar 25, 2019 has no such errors.

@Rurik
Copy link
Owner

Rurik commented Mar 29, 2020

Thank you. I'm troubleshooting this with another issue now. However, I've been unable to reproduce.

What version of Windows are you attempting this on? Is there a .csv file that is produced with data?

@Rurik
Copy link
Owner

Rurik commented Mar 29, 2020

If possible, could you run it with the "-d" option and send the resulting ".log"? Either here or, if it has sensitive info, to brian [@] thebaskins [.] com

@VikramKharvi
Copy link
Author

I ran Noriben.py in Windows 8 and Windows 10 latest build. There is .csv file being generated from procmon but I could see no data going from csv to parse in the parse_csv() function. I did try with different methods but would take more time in order to recreate it again.

@stryker2k2
Copy link

stryker2k2 commented Apr 2, 2020

Howdy. I am having the same issue. Likewise, it seems that Issue #4 from about 5 years ago is very similar to this issue.

The truth is that... this is the BEST Malware Analysis Tool I've ever used and I can't imagine my life without it. Please fix and keep up the good work... FOREVER.

@stryker2k2
Copy link

I've 'fixed' mine.

I moved 'ProcmonConfiguration.pmc' from '/Noriben/filters' to '/Noriben' then re-ran.

All seems to be working fine now.

@VikramKharvi
Copy link
Author

It worked for me too. Kindly update same in the main repo and close the issue

@Rurik
Copy link
Owner

Rurik commented Apr 3, 2020

This is an embarrassing mistake. Thank you to everyone. I did move that file to cleanup the repo and I did not think through when the repo gets cloned and executed. And something that would've been incredibly hard for me to determine without your help.

I will commit the change this weekend, as I have to keep a strict distance from this code and my day job.

@VikramKharvi
Copy link
Author

The CSV report generated by Noriben.py seems to be missing lots of information.

@Rurik
Copy link
Owner

Rurik commented Apr 5, 2020

In more testing, I still cannot reproduce. I created a new VM snapshot without Noriben, copied the repo directly as-is, with the filters folder in place. Windows 10 and Windows 7, Python 3.6 and 3.7.

I suspect there's an environment issue that I did not foresee and cannot determine.

I can push a change to move the file, and to add in the additional debugging code I've added, but I'm hesitant to close this. I suspect it may be a similar issue to #4 where I closed it but am not 100% sure I caught the issue.

It's extreme, but if someone who is exhibiting this problem would not mind a video conference where I can control their VM to test, please let me know by email.

@astrelsky
Copy link

@Rurik I've been facing this problem for a while now and finally had a moment to debug it. Changing the following to if len(field) != 8: appears to have fixed it.

https://github.com/Rurik/Noriben/blob/master/Noriben.py#L1014

@Rurik
Copy link
Owner

Rurik commented Jun 9, 2022

Thank you @astrelsky. That does change a significant check and could break backward compatibility. I will review and test here. That could be the best way forward, or to at least combine a len(field) check against both 8 and 9.

I appreciate you debugging it. It is an odd one that I struggled with, but I will test more against some of my older data sets to ensure it doesn't break.

@astrelsky
Copy link

Thank you @astrelsky. That does change a significant check and could break backward compatibility. I will review and test here. That could be the best way forward, or to at least combine a len(field) check against both 8 and 9.

I appreciate you debugging it. It is an odd one that I struggled with, but I will test more against some of my older data sets to ensure it doesn't break.

No problem. I'm also using a newer procmon version which could be relevant.

Wouldn't it be more effective to check that all the expected fields are there instead of the length? Sure it's more work then a length check but it's only done once and would be more robust. You could log any missing or unexpected fields and skip them accordingly instead of silently producing empty results.

@astrelsky
Copy link

@Rurik I think it may be better to remove this check entirely. If you use csv.DictReader on the csv file opened with encoding utf-8-sig you should be able to replace the row indexing with the row name. It should be more stable this way. If a required row isn't present for some reason you would get a KeyError. It should work even if the rows get moved around in a new version of procmon for example.

@Rurik
Copy link
Owner

Rurik commented Aug 30, 2022

These comments are significant and appreciated, @astrelsky. I agree that DictReader would be the optimal path. Instead of blindly validating and guessing the appropriate fields, they can be referenced by key. I am in a position to do work on Noriben now and will start implementing this change now. That would indeed also remove that blind validation for the number of fields, which could break on different versions of procmon.

@Rurik
Copy link
Owner

Rurik commented Aug 31, 2022

v1.8.7 has been merged. This primarily changes the CSV type. I did regression testing against a smaller set of data than normal (on a laptop in hotel), but everything looks correct.

@astrelsky
Copy link

astrelsky commented Aug 31, 2022

v1.8.7 has been merged. This primarily changes the CSV type. I did regression testing against a smaller set of data than normal (on a laptop in hotel), but everything looks correct.

Cool I'll give it a shot tomorrow morning.

I did notice that the default filter causes all process creations to be filtered out on a newer version of procmon. I'll confirm which filter it was and open a separate issue about it tomorrow. (I think it was * or something weird)

And I totally forgot.

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

No branches or pull requests

4 participants