Bug 1623896 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Original comment by
on
(Hidden by Administrator)
Revision 1 by
on 
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:74.0) Gecko/20100101 Firefox/74.0

Steps to reproduce:

This is the cause of bug 1623057

Sometimes a base64 encoded text/calendar message part may decode to NULL bytes. Example (part):

QkVHSU46VkNBTEVOREFSDQpNRVRIT0Q6UFVCTElTSA0KUFJPRElEOk1pY3Jvc29mdCBFeGNoYW5n
ZSBTZXJ2ZXIgMjAxMA0KVkVSU0lPTjoyLjANCkJFR0lOOlZUSU1FWk9ORQ0KVFpJRDpXLiBFdXJv
cGUgU3RhbmRhcmQgVGltZQAAAAAAAAAAAA0KQkVHSU46U1RBTkRBUkQNCkRUU1RBUlQ6MTYwMTAx

Observe the run of "AAA..." which are NULL bytes.

This leads to corrupted message parts causing e.g. calendar invitations to not show up. But I expect this to affect more attachment types as the issue is pretty generic


Actual results:

From debug tracing this is what happens:

- MimePartBufferRead is called with MimeLeaf_parse_buffer as the read_fn
- This calls MimeLeaf_parse_buffer due to in-memory decoding
- This calls MimeDecoderWrite which forwards to mime_decode_base64_buffer (nothing happend to the data so far)
- Now the base64 encoded data is decoded into the buffer and MimeInlineText_parse_decoded_buffer is called with the buffer (containing embedded NULLs) and the length (real length including the NULLs)
- This is forwarded to mime_LineBuffer

Now comes the important part:
- The buffer is searched for newlines, NULLs are skipped
- Every line found is passed to convert_and_send_buffer with a pointer and length
- This is passed to MimeInlineText_convert_and_parse_line but as it is already valid UTF-8 nothing is actually changed
- Now GatherLine innsSimpleMimeConverterStub.cpp is called, still with the pointer and length
- This does a ssobj->buffer->Append(line) which expects a NULL-terminated string

But because line contains embedded NULLs the line is truncated! This then leads to adding the next line to this line (due to the missing \n) causing garbage to come out of the parser.

I tried to pass the length to Append but then libical errors out, again due to a conversion to NULL terminated string.

What is also surprising: ical.js can deal with embedded zeros just fine. Example (save to index.htm and place next to ical.js):

<html>
<body>
<script src="ical.js"></script>
<script>
var src="QkVHSU46VkNBTEVOREFSDQpNRVRIT0Q6UFVCTElTSA0KUFJPRElEOk1pY3Jvc29mdCBFeGNoYW5n\
ZSBTZXJ2ZXIgMjAxMA0KVkVSU0lPTjoyLjANCkJFR0lOOlZUSU1FWk9ORQ0KVFpJRDpXLiBFdXJv\
cGUgU3RhbmRhcmQgVGltZQAAAAAAAAAAAA0KQkVHSU46U1RBTkRBUkQNCkRUU1RBUlQ6MTYwMTAx\
MDFUMDIwMDAwDQpUWk9GRlNFVEZST006KzAyMDANClRaT0ZGU0VUVE86KzAxMDANClJSVUxFOkZS\
RVE9WUVBUkxZO0lOVEVSVkFMPTE7QllEQVk9LTFTVTtCWU1PTlRIPTEwDQpFTkQ6U1RBTkRBUkQN\
CkJFR0lOOkRBWUxJR0hUDQpEVFNUQVJUOjE2MDEwMTAxVDAxMDAwMA0KVFpPRkZTRVRGUk9NOisw\
MTAwDQpUWk9GRlNFVFRPOiswMjAwDQpSUlVMRTpGUkVRPVlFQVJMWTtJTlRFUlZBTD0xO0JZREFZ\
PS0xU1U7QllNT05USD0zDQpFTkQ6REFZTElHSFQNCkVORDpWVElNRVpPTkUNCkJFR0lOOlZFVkVO\
VA0KT1JHQU5JWkVSO0NOPSJHcnVuZCwgQWxleGFuZGVyIjpNQUlMVE86YWxleGFuZGVyLmdydW5k\
QHR1LWRyZXNkZW4uZGUNClNVTU1BUlk7TEFOR1VBR0U9ZGUtREU6Rk9PDQpEVFNUQVJUO1RaSUQ9\
Vy4gRXVyb3BlIFN0YW5kYXJkIFRpbWUAAAAAAAAAAAA6MjAyMDAzMjJUMTkwMDAwDQpEVEVORDtU\
WklEPVcuIEV1cm9wZSBTdGFuZGFyZCBUaW1lAAAAAAAAAAAAOjIwMjAwMzIyVDIwMDAwMA0KVUlE\
OjQ1ZDQzOGQ4LTM5NTItNDFiYy1hYzAzLTEzNjVhNDljM2YxNw0KQ0xBU1M6UEVSU09OQUwNClBS\
SU9SSVRZOjUNCkRUU1RBTVA6MjAyMDAzMTdUMTcxMTU1Wg0KVFJBTlNQOk9QQVFVRQ0KU1RBVFVT\
OkNPTkZJUk1FRA0KU0VRVUVOQ0U6MA0KTE9DQVRJT047TEFOR1VBR0U9ZGUtREU6DQpYLU1JQ1JP\
U09GVC1DRE8tQVBQVC1TRVFVRU5DRTowDQpYLU1JQ1JPU09GVC1DRE8tT1dORVJBUFBUSUQ6MjEx\
ODM1NjA5Mw0KWC1NSUNST1NPRlQtQ0RPLUJVU1lTVEFUVVM6QlVTWQ0KWC1NSUNST1NPRlQtQ0RP\
LUlOVEVOREVEU1RBVFVTOkJVU1kNClgtTUlDUk9TT0ZULUNETy1BTExEQVlFVkVOVDpGQUxTRQ0K\
WC1NSUNST1NPRlQtQ0RPLUlNUE9SVEFOQ0U6MQ0KWC1NSUNST1NPRlQtQ0RPLUlOU1RUWVBFOjAN\
ClgtTUlDUk9TT0ZULURJU0FMTE9XLUNPVU5URVI6RkFMU0UNCkJFR0lOOlZBTEFSTQ0KREVTQ1JJ\
UFRJT046UkVNSU5ERVINClRSSUdHRVI7UkVMQVRFRD1TVEFSVDotUFQxME0NCkFDVElPTjpESVNQ\
TEFZDQpFTkQ6VkFMQVJNDQpFTkQ6VkVWRU5UDQpFTkQ6VkNBTEVOREFSDQo="
var serialized = atob(src)
var cal = ICAL.parse(serialized)
var comp = new ICAL.Component(cal)
alert(comp.toString())
</script>
</body>
</html>


Expected results:

I think the best solution would be to not use NULL-terminated strings but always pass the length and make the parsers aware of that.

The second best solution would be to remove the embedded zeros in GatherLine (or earlier)

Back to Bug 1623896 Comment 0