Closed Bug 333297 Opened 14 years ago Closed 14 years ago

UMW nsScriptableUnicodeConverter::FinishWithLength overwrites end of buffer

Categories

(Core :: Internationalization, defect, major)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: timeless, Assigned: philor)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity)

Attachments

(1 file, 1 obsolete file)

found by coverity

suggestion: null out finLength-1 instead ?
erm. that should have read:

suggestion: null out finLength-1 instead finLength
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.
Attached patch Fix (obsolete) — Splinter Review
Attachment #217728 - Flags: superreview?(jag)
Attachment #217728 - Flags: review?(smontagu)
Assignee: smontagu → philringnalda
Attachment #217728 - Flags: review?(smontagu) → review+
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).
Attached patch Like so?Splinter Review
"It'll be simple," timeless said. Hmm.
Attachment #217728 - Attachment is obsolete: true
Attachment #217790 - Flags: superreview?(jag)
Attachment #217790 - Flags: review?(smontagu)
Attachment #217728 - Flags: superreview?(jag)
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
Attachment #217790 - Flags: superreview?(jag) → superreview+
Attachment #217790 - Flags: review?(smontagu) → review+
Whiteboard: [checkin needed]
mozilla/intl/uconv/src/nsScriptableUConv.cpp 	1.18
Status: NEW → RESOLVED
Closed: 14 years ago
OS: Linux → All
Hardware: PC → All
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha
You need to log in before you can comment on or make changes to this bug.