bug#24757: 25.1.50; url-cookie.el creates phantom cookie for HttpOnly

bug#24757: 25.1.50; url-cookie.el creates phantom cookie for HttpOnly

[Alain Schneble]

Processing an HTTP response with a Set-Cookie header and HttpOnly
attribute creates a phantom cookie with name HttpOnly.  url-cookie.el
(url-cookie-handle-set-cookie) handles the additional HttpOnly attribute
as the name of an additional cookie, thus interpreting Set-Cookie header
value as it would contain multiple cookies.  This is wrong.  See also
RFC6265 HTTP State Management Mechanism, section 4.1.2.6:
https://www.rfc-editor.org/rfc/rfc6265.txt.

Here's a recipe to reproduce this issue:

- emacs -Q
- Eval the following fragment:
  (let ((file (make-temp-file "CookieHttpOnly")))
    (with-temp-buffer
      (insert
       "(setq url-cookie-storage nil)\n"
       "(setq url-cookie-secure-storage nil)")
      (write-file file))
    (setq url-cookie-file file)
    (url-retrieve-synchronously "https://en.wikipedia.org/wiki/GNU_Guile")
    (url-cookie-write-file)
    (find-file file))
- The visited cookies file should now contain two cookie entries:
  ("en.wikipedia.org"
        [url-cookie "WMF-Last-Access" "21-Oct-2016" "Tue, 22 Nov 2016 12:00:00 GMT" "/" "en.wikipedia.org" t]
        [url-cookie "HttpOnly" nil "Tue, 22 Nov 2016 12:00:00 GMT" "/" "en.wikipedia.org" t])
  => The second cookie entry is not expected.

I would be happy to arrange a patch to solve this issue, but would like
first to discuss which approach to choose:

1. Simply ignore any HttpOnly attribute/flag on a Set-Cookie header
   value.

2. Extend the url-cookie cl-defstruct to contain an additional slot
   HTTPONLY.  Its value would be t if HttpOnly attribute was detected in
   Set-Cookie's header value, nil otherwise.

I could live with both.  What would you prefer?

Alain





In GNU Emacs 25.1.50.1 (x86_64-w64-mingw32)
 of 2016-09-27 built on MYNGB
Repository revision: bbf1ffd7c74bdf3ea766580788f7f4adb98a47f0
Windowing system distributor 'Microsoft Corp.', version 10.0.10586
Configured using:
 'configure --prefix /c/usr/bin/emacs-25.1 --without-imagemagick'

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS

Important settings:
  value of $LANG: DES
  locale-coding-system: cp1252

Major mode: Emacs-Lisp

Minor modes in effect:
  diff-auto-refine-mode: t
  shell-dirtrack-mode: t
  linum-mode: t
  paredit-mode: t
  winner-mode: t
  icomplete-mode: t
  show-paren-mode: t
  display-time-mode: t
  display-battery-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
Mark activated
Insomnium ―― 2014 - Shadows of the Dying Sun [Limited Digipack Edition] (2014) ―― 204-the descent.mp3
Mark set [5 times]
Insomnium ―― 2011 - One For Sorrow (2014) ―― 01 Inertia.mp3
Mark set
Making completion list...
command-execute: Command attempted to use minibuffer while in minibuffer
Quit
Mark set [2 times]
Making completion list...
Quit [2 times]

Features:
(shadow emacsbug debug shr-color color timezone eww mm-url url-queue shr
dom browse-url pcmpl-unix em-unix em-term term ehelp em-script em-prompt
em-ls em-hist em-pred em-glob em-dirs em-cmpl em-basic em-banner
em-alias esh-var esh-io esh-cmd esh-opt esh-ext esh-proc esh-arg
esh-groups eshell esh-module esh-mode esh-util nndoc gnus-dup crm
debbugs-gnu add-log debbugs soap-client xml org-indent
sanityinc-tomorrow-eighties-theme warnings compile autoload tar-mode
lisp-mnt mm-archive url-handlers url-http url-gw url-cache url-auth url
url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf url-util url-parse url-vars pp ace-window ace-jump-mode
advice cl vc-dispatcher vc-svn nxml-uchnm rng-xsd xsd-regexp rng-cmpct
rng-nxml rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt
rng-util rng-pttrn nxml-ns nxml-mode nxml-outln nxml-rap nxml-util
nxml-glyph nxml-enc xmltok apropos tmm artist picture reporter rect
bongo lastfm-submit vc-git diff-mode org-element org-rmail org-mhe
org-irc org-info org-gnus org-docview doc-view subr-x image-mode
org-bibtex bibtex org-bbdb org-w3m shell find-dired gnus-fun jka-compr
misearch multi-isearch eieio-opt speedbar sb-image ezimage dframe
thingatpt nnfolder mailalias smtpmail sendmail nnir qp sort smiley
gnus-cite mail-extr gnus-async gnus-bcklg gnus-ml nndraft nnmh
network-stream nsm auth-source cl-seq starttls gnus-agent gnus-srvr
gnus-score score-mode nnvirtual gnus-msg gnus-art mm-uu mml2015 mm-view
mml-smime smime dig mailcap nntp gnus-cache gnus-sum gnus-group
gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source tls gnutls
utf7 netrc nnoo parse-time gnus-spec gnus-int gnus-range message dired
rfc822 mml mml-sec password-cache epg mm-decode mm-bodies mm-encode
mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils
mailheader gnus-win linum paredit winner ob-ditaa ob-gnuplot org
org-macro org-footnote org-pcomplete pcomplete org-list org-faces
org-entities noutline outline easy-mmode org-version ob-emacs-lisp ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-src ob-keys ob-comint comint
ansi-color ring ob-core ob-eval org-compat org-macs org-loaddefs
format-spec find-func cal-menu calendar cal-loaddefs server ido
icomplete sanityinc-tomorrow-night-theme sanityinc-tomorrow-bright-theme
color-theme-sanityinc-tomorrow paren gnus gnus-ems nnheader gnus-util
mail-utils mm-util help-fns mail-prsvr wid-edit time battery cus-start
cus-load finder-inf ac-js2-autoloads ace-window-autoloads
ace-jump-mode-autoloads bongo-autoloads
color-theme-sanityinc-tomorrow-autoloads company-autoloads
emms-autoloads expand-region-autoloads gnuplot-autoloads
gnuplot-mode-autoloads google-this-autoloads js2-refactor-autoloads
json-mode-autoloads json-reformat-autoloads json-snatcher-autoloads
eieio eieio-core cl-macs multiple-cursors-autoloads
auto-complete-autoloads flycheck-autoloads paredit-autoloads
pkg-info-autoloads epl-autoloads popup-autoloads s-autoloads
skewer-mode-autoloads js2-mode-autoloads simple-httpd-autoloads
solarized-theme-autoloads spacegray-theme-autoloads swift-mode-autoloads
info yasnippet-autoloads zenburn-theme-autoloads package epg-config seq
byte-opt gv bytecomp byte-compile cl-extra help-mode easymenu cconv
edmacro kmacro cl-loaddefs pcase cl-lib time-date mule-util tooltip
eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel
dos-w32 ls-lisp disp-table w32-win w32-vars term/common-win tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment elisp-mode
lisp-mode prog-mode register page menu-bar rfn-eshadow timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame
cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai
tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian
slovak czech european ethiopic indian cyrillic chinese charscript
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote w32notify w32 multi-tty
make-network-process emacs)

Memory information:
((conses 16 1451456 275303)
 (symbols 56 52346 0)
 (miscs 48 3682 1278)
 (strings 32 219721 96665)
 (string-bytes 1 6292673)
 (vectors 16 69870)
 (vector-slots 8 1820989 19661)
 (floats 8 3501 683)
 (intervals 56 148195 1586)
 (buffers 976 118))

bug#24757: 25.1.50; url-cookie.el creates phantom cookie for HttpOnly

[Alain Schneble]

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

Alain Schneble <a.s@realize.ch> writes:

> I would be happy to arrange a patch to solve this issue, but would like
> first to discuss which approach to choose:
>
> 1. Simply ignore any HttpOnly attribute/flag on a Set-Cookie header
>    value.

Following the first approach above, I propose to apply this patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Eliminate-phantom-HttpOnly-cookie-Bug-24757 --]
[-- Type: text/x-patch, Size: 1893 bytes --]

From cf934b9c5d214e0853feef2d8ba42582eb5af5be Mon Sep 17 00:00:00 2001
From: Alain Schneble <a.s@realize.ch>
Date: Sat, 22 Oct 2016 15:43:11 +0200
Subject: [PATCH] Eliminate phantom HttpOnly cookie (Bug#24757)

* lisp/url/url-cookie.el (url-cookie-handle-set-cookie): Remove HttpOnly
attribute from the list of cookie name-value-pairs if it's present in a
Set-Cookie header value.
---
 lisp/url/url-cookie.el | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lisp/url/url-cookie.el b/lisp/url/url-cookie.el
index 6848230..e22bc40 100644
--- a/lisp/url/url-cookie.el
+++ b/lisp/url/url-cookie.el
@@ -245,6 +245,12 @@ url-cookie-handle-set-cookie
   (let* ((args (url-parse-args str t))
 	 (case-fold-search t)
 	 (secure (and (assoc-string "secure" args t) t))
+         ;; HttpOnly attribute was introduced in RFC6265.  Treat it as
+         ;; a cookie name if it appears on the left hand side of a
+         ;; cookie name-value-pair (i.e. HttpCookie=<value>).  Only
+         ;; treat it as HttpOnly flag if it stands alone.
+         (httponly-attribute (assoc-string "httponly" args t))
+         (httponly (and httponly-attribute (not (cdr httponly-attribute))))
 	 (domain (or (cdr-safe (assoc-string "domain" args t))
 		     (url-host url-current-object)))
 	 (current-url (url-view-url t))
@@ -257,7 +263,9 @@ url-cookie-handle-set-cookie
 	 (rest nil))
     (dolist (this args)
       (or (member (downcase (car this)) '("secure" "domain" "expires" "path"))
-	  (setq rest (cons this rest))))
+          ;; Accounts for the special case where HttpOnly is used as cookie name.
+          (and (equal (downcase (car this)) "httponly") httponly)
+          (setq rest (cons this rest))))
 
     ;; Sometimes we get dates that the timezone package cannot handle very
     ;; gracefully - take care of this here, instead of in url-cookie-expired-p
-- 
2.9.1


[-- Attachment #3: Type: text/plain, Size: 76 bytes --]


Could you please consider committing it to the 25.1 branch?

Thanks,
Alain

bug#24757: 25.1.50; url-cookie.el creates phantom cookie for HttpOnly

[Noam Postavsky]

Alain Schneble <a.s@realize.ch> writes:

> Processing an HTTP response with a Set-Cookie header and HttpOnly
> attribute creates a phantom cookie with name HttpOnly.  url-cookie.el
> (url-cookie-handle-set-cookie) handles the additional HttpOnly attribute
> as the name of an additional cookie, thus interpreting Set-Cookie header
> value as it would contain multiple cookies.  This is wrong.  See also
> RFC6265 HTTP State Management Mechanism, section 4.1.2.6:
> https://www.rfc-editor.org/rfc/rfc6265.txt.
>
> Here's a recipe to reproduce this issue:
>
> - emacs -Q
> - Eval the following fragment:
>   (let ((file (make-temp-file "CookieHttpOnly")))
>     (with-temp-buffer
>       (insert
>        "(setq url-cookie-storage nil)\n"
>        "(setq url-cookie-secure-storage nil)")
>       (write-file file))
>     (setq url-cookie-file file)
>     (url-retrieve-synchronously "https://en.wikipedia.org/wiki/GNU_Guile")
>     (url-cookie-write-file)
>     (find-file file))
> - The visited cookies file should now contain two cookie entries:
>   ("en.wikipedia.org"
>         [url-cookie "WMF-Last-Access" "21-Oct-2016" "Tue, 22 Nov 2016 12:00:00 GMT" "/" "en.wikipedia.org" t]
>         [url-cookie "HttpOnly" nil "Tue, 22 Nov 2016 12:00:00 GMT" "/" "en.wikipedia.org" t])
>   => The second cookie entry is not expected.

In emacs-26, as of [1: caa39f495c], the second cookie is not present,
but it looks like it unconditionally drops the HttpOnly attribute (and
all other attributes?).  Is that the right thing?

[1: caa39f495c]: 2017-11-13 23:56:26 +0000
  Fix cookie handling (bug#29282)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=caa39f495c0783dac2d5701100db83ea10f126c0

bug#24757: 25.1.50; url-cookie.el creates phantom cookie for HttpOnly

[Katsumi Yamaoka]

On Wed, 06 Dec 2017 06:46:00 -0500, Noam Postavsky wrote:
[...]
> In emacs-26, as of [1: caa39f495c], the second cookie is not present,
> but it looks like it unconditionally drops the HttpOnly attribute (and
> all other attributes?).  Is that the right thing?

Yes, I believe so.  Not only HttpOnly but also Expires, Max-Age,
etc. are only attributes of the cookie of which the name appeared
at the beginning of the Set-Cookie header.  Sending such ones to
certain web sites would cause an error as I mentioned below.

> [1: caa39f495c]: 2017-11-13 23:56:26 +0000
>   Fix cookie handling (bug#29282)
>   https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=caa39f495c0783dac2d5701100db83ea10f126c0

bug#24757: 25.1.50; url-cookie.el creates phantom cookie for HttpOnly

[Lars Ingebrigtsen]

Alain Schneble <a.s@realize.ch> writes:

> Processing an HTTP response with a Set-Cookie header and HttpOnly
> attribute creates a phantom cookie with name HttpOnly.  url-cookie.el
> (url-cookie-handle-set-cookie) handles the additional HttpOnly attribute
> as the name of an additional cookie, thus interpreting Set-Cookie header
> value as it would contain multiple cookies.  This is wrong.  See also
> RFC6265 HTTP State Management Mechanism, section 4.1.2.6:
> https://www.rfc-editor.org/rfc/rfc6265.txt.
>
> Here's a recipe to reproduce this issue:
>
> - emacs -Q
> - Eval the following fragment:
>   (let ((file (make-temp-file "CookieHttpOnly")))
>     (with-temp-buffer
>       (insert
>        "(setq url-cookie-storage nil)\n"
>        "(setq url-cookie-secure-storage nil)")
>       (write-file file))
>     (setq url-cookie-file file)
>     (url-retrieve-synchronously "https://en.wikipedia.org/wiki/GNU_Guile")
>     (url-cookie-write-file)
>     (find-file file))
> - The visited cookies file should now contain two cookie entries:
>   ("en.wikipedia.org"
>         [url-cookie "WMF-Last-Access" "21-Oct-2016" "Tue, 22 Nov 2016 12:00:00 GMT" "/" "en.wikipedia.org" t]
>         [url-cookie "HttpOnly" nil "Tue, 22 Nov 2016 12:00:00 GMT" "/" "en.wikipedia.org" t])
>   => The second cookie entry is not expected.

I'm unable to reproduce this now, and I seem to vaguely remember this
being fixed a while ago?  Are you still seeing this, Alan?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#24757: 25.1.50; url-cookie.el creates phantom cookie for HttpOnly

[Noam Postavsky]

fixed 24757 26.1
unarchive 29282
forcemerge 29282 24757
quit

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Alain Schneble <a.s@realize.ch> writes:
>> - The visited cookies file should now contain two cookie entries:
>>   ("en.wikipedia.org"
>>         [url-cookie "WMF-Last-Access" "21-Oct-2016" "Tue, 22 Nov 2016 12:00:00 GMT" "/" "en.wikipedia.org" t]
>>         [url-cookie "HttpOnly" nil "Tue, 22 Nov 2016 12:00:00 GMT" "/" "en.wikipedia.org" t])
>>   => The second cookie entry is not expected.
>
> I'm unable to reproduce this now, and I seem to vaguely remember this
> being fixed a while ago?  Are you still seeing this, Alan?

As far as I can tell, this was fixed along with Bug#29282 in 26.1.