boutell / cgic

cgic: an ANSI C library for CGI Programming
Other
249 stars 96 forks source link

Cookies: set HttpOnly flag #9

Closed zackvolz closed 4 years ago

zackvolz commented 5 years ago

This issue came up in a recent security audit. According to this source, it is highly recommended to use this flag:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies

"To prevent cross-site scripting (XSS) attacks, HttpOnly cookies are inaccessible to JavaScript's Document.cookie API; they are only sent to the server. For example, cookies that persist server-side sessions don't need to be available to JavaScript, and the HttpOnly flag should be set."

The change could be made optional, but that would require an API change, like adding a boolean parameter to cgiHeaderCookieSetString().

boutell commented 5 years ago

This can't suddenly always be set as that could be a bc break, although I agree with you that it is a valuable practice. It needs to be optional.

On Tue, Apr 9, 2019 at 4:39 AM Jean-Christophe Zeus < notifications@github.com> wrote:

This issue came up in a recent security audit. According to this source, it is highly recommended to use this flag:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies

"To prevent cross-site scripting (XSS) attacks, HttpOnly cookies are inaccessible to JavaScript's Document.cookie API; they are only sent to the server. For example, cookies that persist server-side sessions don't need to be available to JavaScript, and the HttpOnly flag should be set."

The change could be made optional, but that would require an API change, like adding a boolean parameter to cgiHeaderCookieSetString().

You can view, comment on, or merge this pull request online at:

https://github.com/boutell/cgic/pull/9 Commit Summary

  • Cookies: set HttpOnly flag

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/boutell/cgic/pull/9, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB9ffQanckNa-RtI4fQDnuyMiSkXUPcks5vfFGzgaJpZM4cj7uR .

--

Thomas Boutell, Chief Software Architect P'unk Avenue | (215) 755-1330 | punkave.com

zackvolz commented 5 years ago

Possible solution: the existing function, cgiHeaderCookieSetString(), is kept so the API stays the same. It's refactored into a macro, though, which transforms into a call to the new function cgiHeaderCookieSetStringOptions() - not very happy with the name - with the options parameter set to zero. The latter is a bitmask of enums.

boutell commented 4 years ago

I like it, but let's call it cgiHeaderCookieSet, and deprecate cgiHeaderCookieSetString in the documentation.

cgiHeaderCookieSetString shouldn't become a macro because that would impact anyone passing it by reference, it can just be a wrapper function that invokes the other.

boutell commented 4 years ago

Thanks! Would you like to also submit a PR on the documentation?

zackvolz commented 4 years ago

OK, no problem. Thanks for accepting my pull request!