[issue30458] CRLF Injection in httplib

[Karthikeyan Singaravelan]

Karthikeyan Singaravelan  added the comment:

See also https://bugs.python.org/issue36276 for a similar report. I think it's 
better to raise an error instead of encoding CRLF characters in URL similar to 
headers.

I feel either of the issue and more preferably issue36276 closed as a duplicate 
of this one. Copy of msg337968 with reference to details about similar report 
in golang : 

For reference an exact report on golang repo : 
https://github.com/golang/go/issues/30794 . This seemed to have been fixed in 
latest golang release 1.12 and commit 
https://github.com/golang/go/commit/829c5df58694b3345cb5ea41206783c8ccf5c3ca . 
The commit introduces a check for CTL characters and throws an error for URLs 
something similar to Python does for headers now at bf3e1c9b80e9.

func isCTL(r rune) bool {
return r < ' ' || 0x7f <= r && r <= 0x9f
}

if strings.IndexFunc(ruri, isCTL) != -1 {
return errors.New("net/http: can't write control character in 
Request.URL")
}

So below program used to work before go 1.12 setting a key on Redis but now it 
throws error : 

package main

import "fmt"
import "net/http"

func main() {
resp, err := http.Get("http://127.0.0.1:6379?\r\nSET test 
failure12\r\n:8080/test/?test=a")
fmt.Println(resp)
fmt.Println(err)
}


➜  go version
go version go1.12 darwin/amd64
➜  go run urllib_vulnerability.go

parse http://127.0.0.1:6379?
SET test failure12
:8080/test/?test=a: net/url: invalid control character in URL

Looking more into the commit there seemed to be a solution towards escaping 
characters with https://github.com/golang/go/issues/22907 . The fix seemed to 
have broke Google's internal tests [0] and hence reverted to have the above 
commit where only CTL characters were checked and raises an error. I think this 
is a tricky bug upon reading code reviews in the golang repo that has around 
2-3 reports with a fix committed to be reverted later for a more conservative 
fix and the issue was reopened to target go 1.13 .

Thanks a lot for the report @ragdoll.guo

[0] 
https://go-review.googlesource.com/c/go/+/159157/2#message-39c6be13a192bf760f6318ac641b432a6ab8fdc8

--
nosy: +xtreak

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30458] CRLF Injection in httplib

[Martin Panter]

Martin Panter  added the comment:

Actually, the CRLF + space can be injected via percent encoding, so just 
dealing with literal CRLFs and spaces wouldn’t be enough. You would have to 
validate the hostname after it is decoded.

urlopen("http://127.0.0.1%0D%0A%20SLAVEOF . . . :6379/")

>>> pprint(conn.recv(300).splitlines(keepends=True))
[b'GET / HTTP/1.1\r\n',
 b'Accept-Encoding: identity\r\n',
 b'Host: 127.0.0.1\r\n',
 b' SLAVEOF . . . :6379\r\n',
 b'Connection: close\r\n',
 b'User-Agent: Python-urllib/2.7\r\n',
 b'\r\n']

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30458] CRLF Injection in httplib

[Martin Panter]

Change by Martin Panter :


--
type:  -> security

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30458] CRLF Injection in httplib

[Martin Panter]

Martin Panter added the comment:

You can also inject proper HTTP header fields (or do multiple requests) if you 
omit the space after the CRLF:

urlopen("http://localhost:8000/ HTTP/1.1\r\nHEADER: INJECTED\r\nIgnore:")

Data sent to the server:
>>> server = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP)
>>> server.bind(("localhost", 8000))
>>> server.listen()
>>> [conn, addr] = server.accept()
>>> pprint(conn.recv(300).splitlines(keepends=True))
[b'GET / HTTP/1.1\r\n',
 b'HEADER: INJECTED\r\n',
 b'Ignore: HTTP/1.1\r\n',
 b'Accept-Encoding: identity\r\n',
 b'User-Agent: Python-urllib/3.5\r\n',
 b'Connection: close\r\n',
 b'Host: localhost:8000\r\n',
 b'\r\n']

Issue 14826 is already open about how “urlopen” handles spaces, and there is a 
patch in Issue 13359 that proposes to also encode newline characters. But if 
the CRLF or header injection is a security problem, then 2.7 etc could be 
changed to raise an exception (like Issue 22928), or to do percent encoding.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30458] CRLF Injection in httplib

[Xiang Zhang]

Xiang Zhang added the comment:

Looking at the code and the previous issue #22928, CRLF immediately followed by 
a tab or space (obs-fold: CRLF 1*( SP / HTAB )) is a valid part of a header 
value so the regex deliberately ignore them.

So it looks right to me the url given doesn't raise the same exception as the 
url without spaces, though the given url seems malformed.

--
nosy: +martin.panter, serhiy.storchaka, xiang.zhang

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30458] CRLF Injection in httplib

[Orange]

New submission from Orange:

Hi, the patch in CVE-2016-5699 can be broke by an addition space.
http://www.cvedetails.com/cve/CVE-2016-5699/
https://hg.python.org/cpython/rev/bf3e1c9b80e9
https://hg.python.org/cpython/rev/1c45047c5102

import urllib, urllib2

urllib.urlopen('http://127.0.0.1\r\n\x20hihi\r\n :11211')
urllib2.urlopen('http://127.0.0.1\r\n\x20hihi\r\n :11211')

--
components: Library (Lib)
messages: 294360
nosy: orange
priority: normal
severity: normal
status: open
title: CRLF Injection in httplib
versions: Python 2.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com