bugGNU Wget - Bugs: bug #51576, wget incorrectly uses strncmp()...

 
 

bug #51576: wget incorrectly uses strncmp() and similar in many places

Submitter:  None
Submitted:  Tue 25 Jul 2017 09:21:45 PM UTC
   
 
Category:  Code Architecture Severity:  3 - Normal
Priority:  5 - Normal Status:  None
Privacy:  Public Assigned to:  None
Originator Name:  Derek Originator Email:  -email is unavailable-
Open/Closed:  Open Release:  1.19
Operating System:  None Reproducibility:  Every Time
Fixed Release:  None Planned Release:  None
Regression:  None Work Required:  None
Patch Included:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Fri 28 Jul 2017 02:41:59 PM UTC, comment #1: 

Thanks for pointing out this generic type of error.

The NTLM check is done a bit sloppy, but I can't see no harm here except that the authentication will fail.

As you say, normally strncmp and strcasencmp are just used to check if a string begins with a certain sequence.

The code found by you in http.c seems indeed to be wrong. 'type' is 0-terminated and there is no reason to not use strcasecmp() (and it should be c_strcasecmp here as well).

I'll skim the code to see if there is more.

Tim Ruehsen <rockdaboot>
Group administrator
Tue 25 Jul 2017 09:21:45 PM UTC, original submission:  

Throughout the code, wget uses strncmp(), strncasecmp(), and similar incorrectly.  For example, in http-ntlm.c (abbreviated):

/* return true on success, false otherwise */
bool
ntlm_input (struct ntlmdata *ntlm, const char *header)
{
  if (0 != strncmp (header, "NTLM", 4))
    return false;

  header += 4;
  while (*header && c_isspace(*header))
    header++;

  if (*header)
    {

      ssize_t size;
      char buffer = (char ) alloca (strlen (header));

      DEBUGP (("Received a type-2 NTLM message.\n"));

      size = wget_base64_decode (header, buffer);
      if (size < 0)
        return false;
...

If header is something like "NTLMQWIK" this code will incorrectly treat this as if header was "NTLM" and try to parse "QWIK" as base64.

In this case, the bug looks to be harmless.  However this kind of error occurs in many places in the code, although not all of its uses are incorrect.  I have not made any effort to audit the code further to determine if any such misuses are exploitable.

Other places where strn*cmp() functions are misused include wherever content types are compared, e.g.:

http.c:      0 == strncasecmp (type, TEXTHTML_S, strlen (TEXTHTML_S)) ||

(e.g. "text/html_is_not_this_type" will incorrectly match).

Wherever these functions are used, the code should be carefully audited, and except when comparing a prefix of a string is actually what is intended, strn*cmp should be replaced by str*cmp().

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rockdaboot (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    No changes have been made to this item

    Back to the top

    Powered by Savane 3.12.
    Corresponding source code