LinuxQuestions.org
Welcome to the most active Linux Forum on the web.
Go Back   LinuxQuestions.org > Forums > Non-*NIX Forums > Programming
User Name
Password
Programming This forum is for all programming questions.
The question does not have to be directly related to Linux and any language is fair game.

Notices


Reply
  Search this Thread
Old 12-09-2011, 11:55 PM   #1
hydraMax
Member
 
Registered: Jul 2010
Location: Skynet
Distribution: Debian + Emacs
Posts: 467
Blog Entries: 60

Rep: Reputation: 51
use of "readline" in conky source code


I've recently gotten into the kick of inspecting source code on my system for bugs and vulnerabilities. Picked conky to start with, basically just because I thought it would be smaller and easier to understand. Anyway, I ran flawfinder on it, and the first thing that popped up was this:

Code:
./core.c:99:  [5] (race) readlink:
  This accepts filename arguments; if an attacker can move those files
  or change the link content, a race condition results.  Also, it does not
  terminate with ASCII NUL. Reconsider approach.
Of course, not all flawfinder reports are real problems, so I went to the source, and found it in the function dev_name:

Code:
/* strip a leading /dev/ if any, following symlinks first                                                        
 *                                                                                                               
 * BEWARE: this function returns a pointer to static content                                                     
 *         which gets overwritten in consecutive calls. I.e.:                                                    
 *         this function is NOT reentrant.                                                                       
 */
const char *dev_name(const char *path)
{
    static char buf[255];   /* should be enough for pathnames */
    ssize_t buflen;

    if (!path)
        return NULL;

#define DEV_NAME(x) \
  x != NULL && strlen(x) > 5 && strncmp(x, "/dev/", 5) == 0 ? x + 5 : x
    if ((buflen = readlink(path, buf, 254)) == -1)
        return DEV_NAME(path);
    buf[buflen] = '\0';
    return DEV_NAME(buf);
#undef DEV_NAME
}
So, it appears that the function dev_name is used in the rest of the code to pre-process a file path argument that is supposed to point to a dev file. For the most part all the function is supposed to do is strip "/dev/" from the file name; however, it also calls the readlink function to determine if the path refers to a symbolic link, in which case dev_name will return the file the link points toward rather than simply the modified version of the original path.

So, a few concerns come to mind:

1. Race condition? Is this a good place to be calling readlink?
2. Necessary? What is wrong with simply using the path to the symbolic link?
3. Bad error handling? Notice that, if readlink returns -1 (error) then dev_name simply forgets about the symbolic link and returns the modified version of the original path. However, READLINK(2) indicates that there are a quite a number of reasons that a readlink call might fail:

Code:
RETURN VALUE
       On  success,  readlink() returns the number of bytes placed in buf.  On error, -1 is returned and errno is set to indi‐
       cate the error.

ERRORS
       EACCES Search permission is denied for a component of the path prefix.  (See also path_resolution(7).)

       EFAULT buf extends outside the process's allocated address space.

       EINVAL bufsiz is not positive.

       EINVAL The named file is not a symbolic link.

       EIO    An I/O error occurred while reading from the file system.

       ELOOP  Too many symbolic links were encountered in translating the pathname.

       ENAMETOOLONG
              A pathname, or a component of a pathname, was too long.

       ENOENT The named file does not exist.

       ENOMEM Insufficient kernel memory was available.

       ENOTDIR
              A component of the path prefix is not a directory.
So it seems like we are just "moving on" under the assumption that the file is not a symbolic link, when there could be some other problem. Maybe would should be exiting the program, or logging an error, or something?

Anyway, before I go throwing my ignorant bug reports at the developer, I was curious for feedback.
 
Old 12-12-2011, 06:12 PM   #2
weibullguy
ReliaFree Maintainer
 
Registered: Aug 2004
Location: Kalamazoo, Michigan
Distribution: Slackware 14.2
Posts: 2,815
Blog Entries: 1

Rep: Reputation: 261Reputation: 261Reputation: 261
From the Conky home page --> Join us in #conky on irc.freenode.net to discuss Conky. You probably missed that .

Why don't you go there and see what they say. They wrote the code, maybe they have some insight.
 
Old 12-13-2011, 01:07 AM   #3
hydraMax
Member
 
Registered: Jul 2010
Location: Skynet
Distribution: Debian + Emacs
Posts: 467

Original Poster
Blog Entries: 60

Rep: Reputation: 51
Quote:
Originally Posted by weibullguy View Post
From the Conky home page --> Join us in #conky on irc.freenode.net to discuss Conky. You probably missed that .

Why don't you go there and see what they say. They wrote the code, maybe they have some insight.
Okay, though my IRC experience is always the same:

1. Ask question
2. Wait for 10 minutes.
3. Ask question again
4. Wait for 2 hours
5. Ask question again
6. Wait for 3 hours
7. Ask question again
8. Log out and uninstall IRC client in frustration.

 
Old 12-13-2011, 10:38 AM   #4
weibullguy
ReliaFree Maintainer
 
Registered: Aug 2004
Location: Kalamazoo, Michigan
Distribution: Slackware 14.2
Posts: 2,815
Blog Entries: 1

Rep: Reputation: 261Reputation: 261Reputation: 261
I agree, some IRC channels can be slow. But, how is that different than asking a question. Waiting two days. Getting an answer you don't seem to like. Now what? Uninstall your web browser in frustration? (I jest, of course).

How about trying the Conky mailing list? How about fixing the code and submitting a patch?

You posted at LQ looking for feedback, so I'm giving my feedback. Your initial post here was thoughtful (IMHO), thus I could conclude that a bug report will be thoughtful as well. I'm simply suggesting you spend a little more time attempting to discuss it with the Conky devs. You may get no where, but it follows the general guidelines found in link #4 in my signature. Making the attempt is one thing that sets a good FLOSS user apart from a noob that submits ignorant bug reports.

Anyway, my two cents.

Last edited by weibullguy; 12-13-2011 at 10:40 AM.
 
  


Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off



Similar Threads
Thread Thread Starter Forum Replies Last Post
looking for "C language algorithms for digital signal processing" source code jf.argentino Programming 18 01-12-2020 01:25 PM
Do i need "RPM Packages" or "SuSe Source code packages" Donati Linux - Software 3 03-19-2009 07:32 PM
How to convert Assembly code to "C" source code ssg14j Programming 2 08-01-2005 12:48 PM
A source close to SCO: "linux code has been copied in to system V" qanopus General 4 06-12-2003 01:02 AM

LinuxQuestions.org > Forums > Non-*NIX Forums > Programming

All times are GMT -5. The time now is 01:51 AM.

Main Menu
Advertisement
My LQ
Write for LQ
LinuxQuestions.org is looking for people interested in writing Editorials, Articles, Reviews, and more. If you'd like to contribute content, let us know.
Main Menu
Syndicate
RSS1  Latest Threads
RSS1  LQ News
Twitter: @linuxquestions
Open Source Consulting | Domain Registration