nccgroup / blackboxprotobuf

Blackbox Protobuf is a set of tools for working with encoded Protocol Buffers (protobuf) without the matching protobuf definition.
MIT License
516 stars 86 forks source link

Extension: BurpSuite hangs on large response data with extension enabled #30

Closed Shra1V32 closed 6 months ago

Shra1V32 commented 8 months ago

The BurpSuite software becomes un-usable when there are large protobuf responses.

My Suggestion is to either don't show up in the intercept after certain length or show somewhere else in the BurpSuite.

Great work on the tool, Kudos to your efforts!

rwinkelmaier-ncc commented 8 months ago

Hi!

Thanks for the feedback. Do you have an example request? If not, what size is the message that's triggering the issue?

I'm curious if the issue is a certain type, sheer size or complexity. Or maybe there's some tweaks that could be made to be more efficient.

Do you have many saved message types in Burp? IIRC it tries to decode the payload for each one, so that might make it worse.

In the meantime, you can could modify the detect_protobuf function in user_funcs.py. If that function returns false, it will not try to decode the request. You could check for certain URLs or just check the length there.

Shra1V32 commented 8 months ago

The response is around 5MB, (90k lines) can't share it because it includes the personal details. I'd say, It's not a time complexity issue, because manual decoding takes almost about 1 second. Yes, It has many types. I'll definitely try contributing to this project, but only after my exams.

Meanwhile, If it comes out to be really a time complexity issue, then we can try something new, Mojo Language. Let me know your views on this. Thank you.

Shra1V32 commented 7 months ago

Hi @rwinkelmaier-ncc, Please check the PR and let me know if any changes are to be made.

rwinkelmaier-ncc commented 7 months ago

Hi,

Thanks for the PR. I like the approach of backgrounding instead of limits on the size of messages. It would probably be good to have for smaller messages as well, just to keep the UI more responsive. However, I do want to be careful with the implementation and make sure everything plays nicely with threads. I'll have to think a bit on the right approach and does some research and testing.

Shra1V32 commented 7 months ago

Hi again @rwinkelmaier-ncc, Sure you can!

I've been using this implementation for more than 14 days now, upon my usage, I can say that this is working well as expected. It's just that the string "Please Wait..." string shows up even when the decoding fails. If we were handle this simple thing - then that'd make it complete. Another major issue we have here is the memory usage surges up suddenly to 1.5-1.8 GB only because of the large message.

Thanks once again.

rwinkelmaier-ncc commented 7 months ago

Hey, sorry for the delay. I've had some chance to work on the extension this week, but have been slowly working through how to best implement threading. I want to make sure there's no race conditions for access the shared typedefs or when the get/setMessage functions are called. I also want to make sure threads are cleaned up when the extension unloads and need to decide how to handle threads when the user switches away from the tab, whether to keep going in the background or kill the thread.

However, I dipped into the performance testing a little bit and was able to identify one spot in the decoding where a misnamed variable significantly slowed down decoding for messages with a large number of values and would probably blow up the memory as you described.

If you still have the test case handy, would you be able to retest with the latest version? I haven't pushed to pypi yet, but the updates should be on the github.

Shra1V32 commented 6 months ago

Hello again, Sure I can try them. I'll report you back soon.

Shra1V32 commented 6 months ago

Could you confirm if my PR is still needed and if there are any minor changes to commit?

rwinkelmaier-ncc commented 6 months ago

I still want to implement the threading and can build off your PR for that

Shra1V32 commented 6 months ago

My recent tests with the latest commits show little to no significant change in memory usage.

rwinkelmaier-ncc commented 6 months ago

Hi, I just pushed a commit to the main branch which should background the primary decoding process and I think handles most of the edge cases. I think github didn't automatically mark the PR as merged because I had done a rebase.

Hopefully I didn't break any functionality with the refactor. Let me know if you run into any issues.

You may still run into a hung UI if there are any named typedefs, since it will automatically try to decode the message with each named typedef in order to filter the list and that's not backgrounded right now. I have an idea for a way to do faster filtering without trying to decode the message, but that will have to come a bit later.