Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UriProtocol function is case sensitive #4461

Closed
wixbot opened this issue Jul 1, 2014 · 5 comments
Closed

UriProtocol function is case sensitive #4461

wixbot opened this issue Jul 1, 2014 · 5 comments
Milestone

Comments

@wixbot
Copy link

wixbot commented Jul 1, 2014

One of the libs used thoughout the sdk UriUtil.cpp, the UriProtocol function takes a LPCWSTR and sets an out param that is a URI_PROCOTOL enum. It is comparing specific characters without consideration for case. Specifically the comparison for an HTTPS uri is testing for "httpS".

The expected result of this function should be to compare and match the scheme section of a Uri without regard to case and not compare in a manner where it may access outside the bounds of the given string.

This bug was found by creating a bundle installer with a .NET prereq using the BalExtension, then overriding the WixMbaPrereqLicenseUrl to a value that started with "https://". At runtime a link is shown in the installer, but clicking on the link produces the Error 0x80070002: Failed to launch URL to EULA.

Originally opened by amoravec

@wixbot
Copy link
Author

wixbot commented Jul 2, 2014

We evaluated the out of bounds concern raised in this bug and that is not an issue as long as the strings are null terminated (the SAL annotation on the function suggests they should be). Each character evaluation is AND'd together so if the string is "too short" the null character (\0) will be evaluated and short circuit any remaining evaluations outside of the bounds of the string.

However, the title of the bug is correct. The UriProtocol function in uriutil.cpp is case sensitive.

Originally posted by firegiantco

@wixbot
Copy link
Author

wixbot commented Jul 2, 2014

@firegiant, You are absolutely correct about the null terminated string being short-circuited in the comparison. I've updated the description accordingly to avoid further confusion.

Originally posted by amoravec

@wixbot
Copy link
Author

wixbot commented Jul 10, 2014

Release changed from v4.x to v3.x

@wixbot
Copy link
Author

wixbot commented Aug 15, 2014

Originally changed by amoravec
AssignedTo set to amoravec

@wixbot
Copy link
Author

wixbot commented May 24, 2015

Should be fixed in next build.

Originally posted by robmen
Release changed from v3.x to v3.10
Resolution set to fixed
Status changed from Open to Resolved

@wixbot wixbot added this to the v3.10 milestone Dec 20, 2015
@wixbot wixbot closed this as completed Dec 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant