1435923 - The logging of remote lookup protocol buffer is truncate

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, enhancement, P3)

Description

[François Marier [:francois]]

It looks like we don't submit the redirection URLs when we perform a application reputation lookup against the Google service:

Starting application reputation check [query=0x7fbfa42a80e0]
Created pending lookup [this = 0x7fbfa5c185e0]
PendingLookup::GetStrippedSpec(): URL stripped to 'http://localhost/francois/1434741/content.exe' [this = 0x7fbfa5c185e0]
ApplicationReputation: Got 1 redirects
PendingLookup::GetStrippedSpec(): URL stripped to 'http://localhost/download' [this = 0x7fbfa5c185e0]
ApplicationReputation: Appending redirect http://localhost/download
...
Suggested filename: content.exe [this = 0x7fbfa5c185e0]
Sending remote query for application reputation [this = 0x7fbfa5c185e0]
PendingLookup::GetStrippedSpec(): URL stripped to 'http://localhost/francois/1434741/content.exe' [this = 0x7fbfa5c185e0]
Suggested filename: content.exe [this = 0x7fbfa5c185e0]
Got unsigned binary for remote application reputation check [this = 0x7fbfa5c185e0]
Serialized protocol buffer [this = 0x7fbfa5c185e0]: (length=192) 
-http://localhost/francois/1434741/content.exe"
 
 �p��J����)]��j�N������WQ~�"1
-http://localhost/francois/1434741/content.exe
Destroying pending DB lookup [this = 0x7fbfa42cc8c0]
Application Reputation verdict is 1, obtained in 242.345798 ms [this = 0x7fbfa5c185e0]
Application Reputation check failed, blocking bad binary [this = 0x7fbfa5c185e0]

I would have expected the serialized protobuf to contain "http://localhost/download".

Note: this test case is a simple redirect from http://localhost/download to http://localhost/francois/1434741/content.exe.

Comment 1

[Dimi Lee [:dimi]]

The redirect URLs are not shown in the console because the protocol buffer contains '\x00'.
Print with serialized.c_str()[1] will be cut.
I have verified the protocol buffer contains the information we set.

I have also written a patch to verify the remote lookup protocol buffer stuff, but I will submit the patch in Bug 1190020 because it will also verify the sha256 hash.

[1] https://searchfox.org/mozilla-central/rev/eef79962ba73f7759fd74da658f6e5ceae0fc730/toolkit/components/reputationservice/ApplicationReputation.cpp#1499

Comment 2

[François Marier [:francois]]

Ah, we should probably fix the logging code then. This is bound to confuse someone else in the future :)

Comment 3

[Dimi Lee [:dimi]]

(In reply to François Marier [:francois] from comment #2)
> Ah, we should probably fix the logging code then. This is bound to confuse
> someone else in the future :)

Agree!

Comment 4

[Dimi Lee [:dimi]]

Attachment: Bug 1435923 - Fix truncated logging of remote lookup protocol buffer. r?gcp

Fix the truncate issue when output the protocol buffer

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff26ee3e478c
Fix truncated logging of remote lookup protocol buffer. r=gcp

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/ff26ee3e478c