A potential bug of use of uninitialised variable

[wurongxin1987]

In InfoScreen.c file, there is a potential use of uninitialised variable bug. In Line 133, it calls the function getmouse(&mevent) and mevent may be uninitialised after the function return. As we can see below, the library function getmouse would return ERR without initialising its parameter aevent (when the path condition at Line 1761-1764 is false, it will directly return ERR and not initialise aevent). Then, in the file InfoScreen.c, at Line 133, the path condition is false, and the Line 133-136 will not be executed. However, the Line 137, "if (mevent.y == LINES - 1)" will be executed, and mevent.y is not initialised at all. I guess there is a missing "else" in Line 137, which is the root cause of this bug.

Would you help to check whether this is a true bug? Thanks very much.

[setharnold]

If nothing else the indentation in this block is misleading:

https://github.com/hishamhm/htop/blob/b7b4200f854f667a917b7da8f92b3e0426131bd7/InfoScreen.c#L131

      if (ch == KEY_MOUSE) {
         MEVENT mevent;
         int ok = getmouse(&mevent);
         if (ok == OK)
            if (mevent.y >= panel->y && mevent.y < LINES - 1) {
               Panel_setSelected(panel, mevent.y - panel->y + panel->scrollV);
               ch = 0;
            } if (mevent.y == LINES - 1)
               ch = IncSet_synthesizeEvent(this->inc, mevent.x);
      }

[wurongxin1987]

I think the correct code snippet should be:

    if (ch == KEY_MOUSE) {
         MEVENT mevent;
         int ok = getmouse(&mevent);
         if (ok == OK)
            if (mevent.y >= panel->y && mevent.y < LINES - 1) {
               Panel_setSelected(panel, mevent.y - panel->y + panel->scrollV);
               ch = 0;
            } else if (mevent.y == LINES - 1)
               ch = IncSet_synthesizeEvent(this->inc, mevent.x);
      }

or:

   if (ch == KEY_MOUSE) {
         MEVENT mevent;
         int ok = getmouse(&mevent);
         if (ok == OK) {
                if (mevent.y >= panel->y && mevent.y < LINES - 1) {
                   Panel_setSelected(panel, mevent.y - panel->y + panel->scrollV);
                   ch = 0;
                } 
                if (mevent.y == LINES - 1)
                   ch = IncSet_synthesizeEvent(this->inc, mevent.x);
         }
      }

The original code is not just a code formatting issue. It is a bug about the use of unitialised variable. Please read my description above. This would be a security bug.

[dodona2]

it becomes initialized quickly by a call by reference: getmouse(&mevent);

[wurongxin1987]

it becomes initialized quickly by a call by reference: getmouse(&mevent);

Be noted that, getmouse can fail to initialize mevent. Please read the following code of getmouse.

When the path condition in Line 1761-1764 is false, it will directly return ERR without initializing mevent. Then, the following code snippet will be buggy.

        int ok = getmouse(&mevent);
        if (ok == OK)
            if (mevent.y >= panel->y && mevent.y < LINES - 1) {
               Panel_setSelected(panel, mevent.y - panel->y + panel->scrollV);
               ch = 0;
            } 
         if (mevent.y == LINES - 1)
             ch = IncSet_synthesizeEvent(this->inc, mevent.x);

[wurongxin1987]

It seems no one is tracking this bug. Can anyone who can check whether the pull request https://github.com/hishamhm/htop/pull/883 fixed this issue? If so, can anyone merge it? Thanks.

[cpaelzer]

This might not be the most critical error, but any cleanup is good right - therefore I wanted to ping if there is any update in considering the suggested change?

[wurongxin1987]

@hishamhm Can you help to check this bug? Thanks.