Opened 5 years ago

Closed 4 years ago

#10231 closed bug (fixed)

Fixed uninitialized array in Terminal.

Reported by: Ezodev Owned by: siarzhuk
Priority: normal Milestone: R1
Component: Applications/Terminal Version: R1/Development
Keywords: gci2013 Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All



Attachments (1)

0001-Fixed-unint.-array-CID-743879.patch (630 bytes) - added by Ezodev 5 years ago.

Download all attachments as: .zip

Change History (7)

Changed 5 years ago by Ezodev

comment:1 Changed 5 years ago by Ezodev

Has a Patch: set

comment:2 Changed 5 years ago by anevilyak

Owner: changed from jackburton to siarzhuk
Status: newassigned

comment:3 Changed 5 years ago by umccullough

This appears to be an attempt at fixing the symptom reported by Coverity - when it seems there's a larger underlying issue here. It seems that param[] shouldn't be used here without being set properly further up in the code - initializing it to {0} (if that works - which I don't know) just hides the problem rather than resolving it properly.

Since I'm unclear what should have happened prior to the use of param[] - I'm not sure I can say how it should be fixed - this should probably just be marked as "bug" in Coverity and someone like siarzhuk can look at it.

comment:4 Changed 5 years ago by anevilyak

From a cursory glance at the CID's analysis and the TermParse code, I believe the correct solution here would actually be something more like memset(param, DEFAULT, sizeof(param));, but I'm going to have to defer to someone with greater familiarity with the terminal parsing code for a second opinion.

comment:5 Changed 5 years ago by Ezodev

Yes, I dont know how to handle it properly for 100%. I'm new and I don't know the codebase. But, I've chosen 0 because it's default value (I've expecting that code assumes that this is initialized to 0), and this is better than undefinied values.

comment:6 Changed 4 years ago by pulkomandy

Resolution: fixed
Status: assignedclosed

Initialized the array in hrev48334.

I followed the sequence used by Coverity and it is invalid, it jumps between states in way that the state tables in VPrsTbl.c would not allow (from CASE_ESC_IGNORE to CASE_ESC_DIGIT). The only way to enter ESC_DIGIT is from CASE_CSI_STATE, which sets the first element of the param array to DEFAULT. Then following elements of the array are initialized each time nparam is incremented.

Just to be safe, the array is now cleared once when entering the function.

Note: See TracTickets for help on using tickets.