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 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

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, 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.3