[webkit-reviews] review granted: [Bug 170994] Stop using strcpy() in WebKit::EnvironmentUtilities::stripValuesEndingWithString() : [Attachment 307482] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 170994: Stop using strcpy() in
WebKit::EnvironmentUtilities::stripValuesEndingWithString()
https://bugs.webkit.org/show_bug.cgi?id=170994

Attachment 307482: Patch

https://bugs.webkit.org/attachment.cgi?id=307482&action=review




--- Comment #3 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 307482
  --> https://bugs.webkit.org/attachment.cgi?id=307482
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=307482&action=review

This looks great! But since I'm greedy, what about replacing instances of
"strstr" in that method with "strnstr"?

> Source/WebKit2/Platform/unix/EnvironmentUtilities.cpp:46
> +    size_t environmentValueLength = strlen(environmentValue) + 1;

What about adding some "ASSERT_WITH_SECURITY_IMPLICATIONS" that our later
string iteration does not exceed this amount? (e.g., the various 'match'
results?)

> Source/WebKit2/Platform/unix/EnvironmentUtilities.cpp:77
> +	       strlcpy(componentStart, match + searchLength,
environmentValueLength - (componentStart - environmentValue));

I think this is good, but we should also fix "strstr" -> "strnstr"

> Source/WebKit2/Platform/unix/EnvironmentUtilities.cpp:85
>	   match = strstr(componentStart, searchValueWithColon);

I think these should be "strnstr"