1488452 - Avoid writing past the logical length of a string in FileUtilsWin.h

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Henri Sivonen (:hsivonen)]

The call to GetLogicalDriveStringsW first writes and then sets length.

Comment 1

[Henri Sivonen (:hsivonen)]

I misunderstood the semantics of GetLogicalDriveStringsW. There's still a bug somewhere around here.

Comment 2

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f1073d2aa1411a33f06b8316f18f12c46ed2f19

Comment 3

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1488452 - Remove incorrect use of PromiseFlatString in FileUtilsWin.

Also rewrote some code around the actual problem to be more obviously correct
and comprehensible.

MozReview-Commit-ID: FF2hSjQ4U1x

Comment 4

[Henri Sivonen (:hsivonen)]

I took a look of the our other uses of PromiseFlatString and PromiseFlatCString and, fortunately, this seems to be a unique misuse of PromiseFlatString.

Comment 5

[Henri Sivonen (:hsivonen)]

https://searchfox.org/mozilla-central/source/gfx/vr/gfxVROculus.cpp#658 started asserting about non-ASCII. I don't know why it started now, but using AppendPrintf to build a Windows file path seems suspicious to begin with.

Comment 6

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #5)
> https://searchfox.org/mozilla-central/source/gfx/vr/gfxVROculus.cpp#658
> started asserting about non-ASCII. I don't know why it started now, but
> using AppendPrintf to build a Windows file path seems suspicious to begin
> with.

Ah, that code also has the write-past-mLength pattern, but I failed to find it on searchfox, because it's #ifdefed out as far as searchfox can tell.

Comment 7

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #6)
> (In reply to Henri Sivonen (:hsivonen) from comment #5)
> > https://searchfox.org/mozilla-central/source/gfx/vr/gfxVROculus.cpp#658
> > started asserting about non-ASCII. I don't know why it started now, but
> > using AppendPrintf to build a Windows file path seems suspicious to begin
> > with.
> 
> Ah, that code also has the write-past-mLength pattern, but I failed to find
> it on searchfox, because it's #ifdefed out as far as searchfox can tell.

Bug 1489462.

Comment 8

[Nathan Froyd [:froydnj]]

Comment on attachment 9006947 [details]
Bug 1488452 - Remove incorrect use of PromiseFlatString in FileUtilsWin.

Nathan Froyd [:froydnj] has approved the revision.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/36ee80fc14ae
Remove incorrect use of PromiseFlatString in FileUtilsWin. r=froydnj

Comment 10

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/36ee80fc14ae