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

 
 

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

Submitted by:  None
Submitted on:  Tue 25 Jul 2017 09:21:45 PM UTC  
 
Category: Code ArchitectureSeverity: 3 - Normal
Priority: 5 - NormalStatus: None
Privacy: PublicAssigned to: None
Originator Name: DerekOriginator Email: -unavailable-
Open/Closed: OpenRelease: 1.19
Operating System: NoneReproducibility: Every Time
Fixed Release: NonePlanned Release: None
Regression: NoneWork Required: None
Patch Included: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

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>
Project 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, strncmp should be replaced by strcmp().

Anonymous

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    No Changes Have Been Made to This Item

    Back to the top


    Powered by Savane 3.1-cleanup1