php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #60668 Setting user_agent can send other headers
Submitted: 2012-01-06 10:08 UTC Modified: 2012-03-03 20:07 UTC
Votes:2
Avg. Score:4.0 ± 1.0
Reproduced:0 of 0 (0.0%)
From: vrana@php.net Assigned:
Status: Not a bug Package: HTTP related
PHP Version: 5.4.0RC5 OS: Irrelevant
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: vrana@php.net
New email:
PHP Version: OS:

 

 [2012-01-06 10:08 UTC] vrana@php.net
Description:
------------
Setting 'user_agent' INI value to a string containing a newline causes sending a new header. This behavior is even documented: http://php.net/wrappers.http#wrappers.http.example.custom.headers

It is wrong for two reasons:

1. 'user_agent' INI setting should be used only for setting a User-Agent header and not for anything else.

2. It is a potential security risk (header injection) similar to the one fixed in PHP 5.1.2 (but with low impact).

(See also bug #52979 but I believe that I am providing a better reasoning.)

Test script:
---------------
<?php
$_POST['user_agent'] = "Robot\r\nX-Command: delete-all";
ini_set('user_agent', $_POST['user_agent']);
readfile('http://private/service.php');
?>


Expected result:
----------------
Sending just a User-Agent header, not X-Command header.

Actual result:
--------------
Sending User-Agent and X-Command headers.

If http://private/service.php accepts connections only from trusted sources and parses its commands from headers then it will execute the malicious action.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-01-25 10:48 UTC] me at ktamura dot com
vrana: I think this is a pretty bad security issue. Here is a proposed diff as a 
github gist: https://gist.github.com/1675788
 [2012-03-03 20:07 UTC] iliaa@php.net
-Status: Open +Status: Not a bug
 [2012-03-03 20:07 UTC] iliaa@php.net
Sorry, but your problem does not imply a bug in PHP itself.  For a
list of more appropriate places to ask for help using PHP, please
visit http://www.php.net/support.php as this bug system is not the
appropriate forum for asking support questions.  Due to the volume
of reports we can not explain in detail here why your report is not
a bug.  The support channels will be able to provide an explanation
for you.

Thank you for your interest in PHP.

It is up-to the developer to ensure that input supplied by the user is properly 
validated before being transmitted back to the user or external services. In this 
particular case the issue is input validation. You can do the same operation with 
any stream operation by feeding it user data.

For example:

$fp = fsockopen(web_server, 80);
fwrite($fp, $user_input);
...
 [2014-01-22 03:26 UTC] vovan-ve at yandex dot ru
I'm disagree with "Not a bug". What kind of value we pass to 'user_agent' ini option?

>
> Define the user agent for PHP to send.
>

Nothing said. In fact, we pass piece of whole block of headers, not just a value.

RFC2616 says:

    User-Agent      = "User-Agent" ":" 1*( product | comment )
    product         = token ["/" product-version]
    product-version = token
    token           = 1*<any CHAR except CTLs or separators>
    separators      = "(" | ")" | "<" | ">" | "@"
                    | "," | ";" | ":" | "\" | <">
                    | "/" | "[" | "]" | "?" | "="
                    | "{" | "}" | SP | HT

    comment        = "(" *( ctext | quoted-pair | comment ) ")"
    ctext          = <any TEXT excluding "(" and ")">
    quoted-pair    = "\" CHAR

    TEXT           = <any OCTET except CTLs,
                     but including LWS>

    LWS            = [CRLF] 1*( SP | HT )

    OCTET          = <any 8-bit sequence of data>
    CHAR           = <any US-ASCII character (octets 0 - 127)>
    CTL            = <any US-ASCII control character
                     (octets 0 - 31) and DEL (127)>

Ok, looks like end programmer should to pass correct value. But only VALUE, not piece of headers. Yes, problem goes from end programmer, who pass wrong things to 'user_agent', who doesn't know about RFC2616.

CRLF can be found in User-Agent definition only inside `ctext` and only as LWS. So, PHP can check, if 'user_agent' has CRLF without SPACE or TAB after it. PHP doesn't need to check it on every run. One check per run just before first use of 'user_agent' and when changing it with ini_set() is enough to throw a Notice, a Warning or do something else to say end programmer, that he is not right. PHP can even insert SPACE after CRLF in that case and use new value instead of incorrect value.
 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Fri Jul 30 15:01:23 2021 UTC