User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36 Steps to reproduce: [Windows] Use "Restart to update Firefox" from "About Mozilla Firefox" dialog. Paths with updates or FF itself should contain a reparse point. Actual results: Updater.exe calls a function PathContainsInvalidLinks to check update locations. When some path contains a reparse point, the function retrieves the reparse target: switch (buffer->ReparseTag) { case IO_REPARSE_TAG_MOUNT_POINT: reparseTarget = buffer->MountPointReparseBuffer.PathBuffer; break; case IO_REPARSE_TAG_SYMLINK: reparseTarget = buffer->SymbolicLinkReparseBuffer.PathBuffer; break; This is invalid because the target name location inside the PathBuffer is controlled by SubstituteNameOffset and SubstituteNameLength fields, but the code uses fixed offset 0. Expected results: In my case, I have fields like this (let's use python here): >>> SubstituteNameOffset=0x1a >>> SubstituteNameLength=0x22 >>> PrintNameOffset=0 >>> PrintNameLength=0x1a >>> PathBuffer=u"D:\\Users\\User\\??\\D:\\Users\\User" So, in this case the target would be >>> print PathBuffer[SubstituteNameOffset/2:SubstituteNameOffset/2+SubstituteNameLength/2] \??\D:\Users\User

Good catch, thanks for reporting this. Somehow I missed that the print name and the substitute name are not the same thing.

The two standard reparse data buffer structs (SymbolicLinkReparseBuffer and MountPointReparseBuffer) both contain one UTF-16 array called PathBuffer which contains two paths, the "print name" and the "substitute name", with no separator between them. There are also fields in the struct that provide the offset and the length of both those paths (in bytes). I had originally missed that these were separate paths and that the print name will typically not match the substitute name for file system links. This patch corrects that oversight and uses the offsets to correctly check only the substitute name.

Pushed by mhowell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7ee4eac9ae35 Fix determining the correct target path of a reparse point. r=rstrong

Matt, could you request an uplift to beta please? Thanks

Comment on attachment 9014138 [details] Bug 1496094 - Fix determining the correct target path of a reparse point. r=rstrong [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1481907 User impact if declined: Windows Firefox installations located in paths containing directory symlinks will fail to update. I don't have data on that configuration, but it is certainly not common. Is this code covered by automated tests?: No Has the fix been verified in Nightly?: Yes Needs manual test from QE?: Yes If yes, steps to reproduce: - Create a symbolic link (let's call it C:\testlink) to Program Files using the mklink tool. - Install a build of Firefox which has this patch to a path inside C:\testlink - Attempt to update that installation. If the update succeeds without incident, this bug is fixed. List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): This is a small fix to the way we're using a Windows API struct in code that only runs at all under very uncommon configurations. String changes made/needed: N/A

Comment on attachment 9014138 [details] Bug 1496094 - Fix determining the correct target path of a reparse point. r=rstrong Contained fix for a recent P1 regression, uplift approved for 63 beta 13, thanks.

Comment on attachment 9014138 [details] Bug 1496094 - Fix determining the correct target path of a reparse point. r=rstrong [ESR Uplift Approval Request] If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a very low risk patch that fixes potential app update failures which we don't have a way to automatically recover from. User impact if declined: Windows Firefox installations located in paths containing directory symlinks will fail to update. I don't have data on that configuration, but it is certainly not common. Fix Landed on Version: 64 with uplift to 63 Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): This is a small fix to the way we're using a Windows API struct in code that only runs at all under very uncommon configurations. String or UUID changes made by this patch:

Comment on attachment 9014138 [details] Bug 1496094 - Fix determining the correct target path of a reparse point. r=rstrong Feels like one of those issues we could find ourselves dot releasing for if underestimate the impact. Especially given what a simple fix it is, I agree that taking the fix for all affected releases makes sense. Approved for ESR 60.3.

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0 Build ID: 20181008155858 User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0 Build ID: 20181010193149 Verified as fixed on the latest beta build (v63beta13) and also on the ESR build (60.2.3)