some potential security issue for edid-decode

Thu Sep 29 14:48:30 UTC 2016

Hi all,

First of all thank you all for the wonderful work you are during in
X.org and its components. I have been using x.org and it components
forever.

Please make sure to CC me in case anybody replies as I have turned off
general replies from the list as I'm already info. overloaded.

I have been looking at a tool called I-Nex. During course of things I
came across the following -

┌─[shirish at debian] - [~/games/I-Nex] - [10043]
└─[$] flawfinder -Q -c .

Flawfinder version 1.31, (C) 2001-2014 David A. Wheeler.
Number of rules (primarily dangerous function names) in C/C++ ruleset: 169
./JSON/i-nex-edid.c:137:  [2] (buffer) char:
  Statically-sized arrays can be improperly restricted, leading to potential
  overflows or other issues (CWE-119:CWE-120). Perform bounds checking, use
  functions that limit length, or ensure that the size is larger than the
  maximum possible length.
    static char name[4];
./JSON/i-nex-edid.c:153:  [2] (buffer) char:
  Statically-sized arrays can be improperly restricted, leading to potential
  overflows or other issues (CWE-119:CWE-120). Perform bounds checking, use
  functions that limit length, or ensure that the size is larger than the
  maximum possible length.
    const unsigned char empty[3] = { 0, 0, 0 };
./JSON/i-nex-edid.c:211:  [2] (buffer) char:
  Statically-sized arrays can be improperly restricted, leading to potential
  overflows or other issues (CWE-119:CWE-120). Perform bounds checking, use
  functions that limit length, or ensure that the size is larger than the
  maximum possible length.
    static char ret[128];
./JSON/i-nex-edid.c:241:  [2] (buffer) char:
  Statically-sized arrays can be improperly restricted, leading to potential
  overflows or other issues (CWE-119:CWE-120). Perform bounds checking, use
  functions that limit length, or ensure that the size is larger than the
  maximum possible length.
    static unsigned char name[53];
./JSON/i-nex-edid.c:1587:  [2] (buffer) char:
  Statically-sized arrays can be improperly restricted, leading to potential
  overflows or other issues (CWE-119:CWE-120). Perform bounds checking, use
  functions that limit length, or ensure that the size is larger than the
  maximum possible length.
        char buf[3];
./JSON/i-nex-edid.c:1621:  [2] (buffer) char:
  Statically-sized arrays can be improperly restricted, leading to potential
  overflows or other issues (CWE-119:CWE-120). Perform bounds checking, use
  functions that limit length, or ensure that the size is larger than the
  maximum possible length.
        char buf[3];
./JSON/i-nex-edid.c:1683:  [2] (buffer) char:
  Statically-sized arrays can be improperly restricted, leading to potential
  overflows or other issues (CWE-119:CWE-120). Perform bounds checking, use
  functions that limit length, or ensure that the size is larger than the
  maximum possible length.
        char buf[3];
./JSON/i-nex-edid.c:1776:  [2] (misc) open:
  Check when opening files - can an attacker redirect it (via symlinks),
  force the opening of special file type (e.g., device files), move things
  around to create a race condition, control its ancestors, or change its
  contents? (CWE-362).
        if ((fd = open(argv[1], O_RDONLY)) == -1) {
./JSON/i-nex-edid.c:1783:  [2] (misc) open:
  Check when opening files - can an attacker redirect it (via symlinks),
  force the opening of special file type (e.g., device files), move things
  around to create a race condition, control its ancestors, or change its
  contents? (CWE-362).
        if ((fd = open(argv[1], O_RDONLY)) == -1) {
./JSON/i-nex-edid.c:1787:  [2] (misc) open:
  Check when opening files - can an attacker redirect it (via symlinks),
  force the opening of special file type (e.g., device files), move things
  around to create a race condition, control its ancestors, or change its
  contents? (CWE-362).
        if ((ofd = open(argv[2], O_WRONLY)) == -1) {
./JSON/i-nex-edid.c:319:  [1] (buffer) strncat:
  Easily used incorrectly (e.g., incorrectly computing the correct maximum
  size to add) (CWE-120). Consider strcat_s, strlcat, or automatically
  resizing strings.
        strncat((char *)name, (char *)x + 5, 13);
./JSON/i-nex-edid.c:324:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated; if given one it may
  perform an over-read (it could cause a crash if unprotected) (CWE-126).
                      strlen((char *)name)));
./JSON/i-nex-edid.c:1521:  [1] (buffer) read:
  Check buffer boundaries if used in a loop including recursive loops
  (CWE-120, CWE-20).
    i = read(fd, ret + len, size - len);
./JSON/i-nex-edid.c:1576:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated; if given one it may
  perform an over-read (it could cause a crash if unprotected) (CWE-126).
        start = s + strlen(indentation);
./JSON/i-nex-edid.c:1735:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated; if given one it may
  perform an over-read (it could cause a crash if unprotected) (CWE-126).
    for (i = strlen(name); i < 15; i++)

ANALYSIS SUMMARY:

Hits = 15
Lines analyzed = 3128 in approximately 0.39 seconds (7995 lines/second)
Physical Source Lines of Code (SLOC) = 2745
Hits at level = [0]   0 [1]   5 [2]  10 [3]   0 [4]   0 [5]   0
Hits at level+ = [0+]  15 [1+]  15 [2+]  10 [3+]   0 [4+]   0 [5+]   0
Hits/KSLOC at level+ = [0+] 5.46448 [1+] 5.46448 [2+] 3.64299 [3+]   0
[4+]   0 [5+]   0
Dot directories skipped = 7 (--followdotdir overrides)
Minimum risk level = 1
Not every hit is necessarily a security vulnerability.
There may be other security vulnerabilities; review your code!
See 'Secure Programming for Linux and Unix HOWTO'
(http://www.dwheeler.com/secure-programs) for more information.

I filed an issue with the author at https://github.com/eloaders/I-Nex/issues/47

and came to know that this is a clone of the edid-decode package
managed by freedesktop.

I am not really a coder hence can't help. It is also possible that the
errors or warnings may be false positives but that's upto the X.org
developers to see if there is any issue or not.

I did have a cursory look at
https://bugs.freedesktop.org/buglist.cgi?quicksearch=edid-decode but
didn't find anything which corresponds to the warnings highlighted
above.

Maybe somebody can take a look at it. Please let me know if you need
me to share some more info. from my side.

-- 
          Regards,
          Shirish Agarwal  शिरीष अग्रवाल
  My quotes in this email licensed under CC 3.0
http://creativecommons.org/licenses/by-nc/3.0/
http://flossexperiences.wordpress.com
EB80 462B 08E1 A0DE A73A  2C2F 9F3D C7A4 E1C4 D2D8