333297 - UMW nsScriptableUnicodeConverter::FinishWithLength overwrites end of buffer

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[timeless]

found by coverity

suggestion: null out finLength-1 instead ?

Comment 1

[timeless]

erm. that should have read:

suggestion: null out finLength-1 instead finLength

Comment 2

[Simon Montagu :smontagu]

Better to allocate finlength+1. mEncoder->Finish() could write up to finLength bytes to the buffer, so nulling out finLength - 1 would overwrite the last byte.

Comment 3

[Phil Ringnalda (:philor)]

Attachment: Fix

Comment 4

[jag (Peter Annema)]

Comment on attachment 217728 [details] [diff] [review]
Fix

Another option would be to not null terminate at all; you are returning the encoded length after all.

Btw, I noticed that one of the callers of FinishWithLength() doesn't free the returned string.

Also, I would move the aLength assignment into the then-block for consistency (is its value even defined when Finish() fails?), since neither caller will touch either of the out-params if NS_FAILED(rv).

Comment 5

[Phil Ringnalda (:philor)]

Attachment: Like so?

"It'll be simple," timeless said. Hmm.

Comment 6

[jag (Peter Annema)]

Comment on attachment 217790 [details] [diff] [review]
Like so?

It is simple, but then you get reviewers like me who notice these other bits and instead of filing more bugs they wonder out loud if this person's perhaps willing to fix those issues too while they're in there anyway ;-)

sr=jag

Comment 7

[:Gavin Sharp [email: [e-mail]]]

mozilla/intl/uconv/src/nsScriptableUConv.cpp 	1.18