mupen64plus / mupen64plus-core

Core module of the Mupen64Plus project
1.29k stars 257 forks source link

Coverity scan defects founds #975

Closed Narann closed 2 years ago

Narann commented 2 years ago

Coverity scan reports few problems.

It's still unclear of those are valid defects or simply Coverity scan fall into a hole.

Feel free to close the ticket.

Please find the latest report on new defect(s) introduced to mupen64plus-core found with Coverity Scan.

25 new defect(s) introduced to mupen64plus-core found with Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 20 of 25 defect(s)

** CID 381051:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/rdp/rdp_core.c: 89 in read_dpc_regs()

________________________________________________________________________________________________________
*** CID 381051:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/rdp/rdp_core.c: 89 in read_dpc_regs()
83   
84    void read_dpc_regs(void* opaque, uint32_t address, uint32_t* value)
85    {
86        struct rdp_core* dp = (struct rdp_core*)opaque;
87        uint32_t reg = dpc_reg(address);
88   
>>>    CID 381051:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array "dp->dpc_regs" of 8 4-byte elements at element index 16383 (byte offset 65535) using index "reg" (which evaluates to 16383).
89        *value = dp->dpc_regs[reg];
90    }
91   
92    void write_dpc_regs(void* opaque, uint32_t address, uint32_t value, uint32_t mask)
93    {
94        struct rdp_core* dp = (struct rdp_core*)opaque;

** CID 381050:    (STRING_OVERFLOW)

________________________________________________________________________________________________________
*** CID 381050:    (STRING_OVERFLOW)
/src/osal/files_unix.c: 164 in osal_get_shared_filepath()
158        if (secondsearch != NULL && search_dir_file(retpath, secondsearch, filename) == 0)
159            return retpath;
160   
161        /* otherwise check our standard paths */
162        for (i = 0; i < datasearchdirs; i++)
163        {
>>>    CID 381050:    (STRING_OVERFLOW)
>>>    You might overrun the 4096-character fixed-size string "retpath" by copying "filename" without checking the length.
164            if (search_dir_file(retpath, datasearchpath[i], filename) == 0)
165                return retpath;
166        }
167   
168        /* we couldn't find the file */
169        return NULL;
/src/osal/files_unix.c: 156 in osal_get_shared_filepath()
150    const char * osal_get_shared_filepath(const char *filename, const char *firstsearch, const char *secondsearch)
151    {
152        static char retpath[PATH_MAX];
153        int i;
154   
155        /* if caller gave us any directories to search, then look there first */
>>>    CID 381050:    (STRING_OVERFLOW)
>>>    You might overrun the 4096-character fixed-size string "retpath" by copying "filename" without checking the length.
156        if (firstsearch != NULL && search_dir_file(retpath, firstsearch, filename) == 0)
157            return retpath;
158        if (secondsearch != NULL && search_dir_file(retpath, secondsearch, filename) == 0)
159            return retpath;
160   
161        /* otherwise check our standard paths */
/src/osal/files_unix.c: 158 in osal_get_shared_filepath()
152        static char retpath[PATH_MAX];
153        int i;
154   
155        /* if caller gave us any directories to search, then look there first */
156        if (firstsearch != NULL && search_dir_file(retpath, firstsearch, filename) == 0)
157            return retpath;
>>>    CID 381050:    (STRING_OVERFLOW)
>>>    You might overrun the 4096-character fixed-size string "retpath" by copying "filename" without checking the length.
158        if (secondsearch != NULL && search_dir_file(retpath, secondsearch, filename) == 0)
159            return retpath;
160   
161        /* otherwise check our standard paths */
162        for (i = 0; i < datasearchdirs; i++)
163        {
/src/osal/files_unix.c: 158 in osal_get_shared_filepath()
152        static char retpath[PATH_MAX];
153        int i;
154   
155        /* if caller gave us any directories to search, then look there first */
156        if (firstsearch != NULL && search_dir_file(retpath, firstsearch, filename) == 0)
157            return retpath;
>>>    CID 381050:    (STRING_OVERFLOW)
>>>    You might overrun the 4096-character fixed-size string "retpath" by copying "secondsearch" without checking the length.
158        if (secondsearch != NULL && search_dir_file(retpath, secondsearch, filename) == 0)
159            return retpath;
160   
161        /* otherwise check our standard paths */
162        for (i = 0; i < datasearchdirs; i++)
163        {
/src/osal/files_unix.c: 156 in osal_get_shared_filepath()
150    const char * osal_get_shared_filepath(const char *filename, const char *firstsearch, const char *secondsearch)
151    {
152        static char retpath[PATH_MAX];
153        int i;
154   
155        /* if caller gave us any directories to search, then look there first */
>>>    CID 381050:    (STRING_OVERFLOW)
>>>    You might overrun the 4096-character fixed-size string "retpath" by copying "firstsearch" without checking the length.
156        if (firstsearch != NULL && search_dir_file(retpath, firstsearch, filename) == 0)
157            return retpath;
158        if (secondsearch != NULL && search_dir_file(retpath, secondsearch, filename) == 0)
159            return retpath;
160   
161        /* otherwise check our standard paths */
/src/osal/files_unix.c: 164 in osal_get_shared_filepath()
158        if (secondsearch != NULL && search_dir_file(retpath, secondsearch, filename) == 0)
159            return retpath;
160   
161        /* otherwise check our standard paths */
162        for (i = 0; i < datasearchdirs; i++)
163        {
>>>    CID 381050:    (STRING_OVERFLOW)
>>>    You might overrun the 4096-character fixed-size string "retpath" by copying "datasearchpath[i]" without checking the length.
164            if (search_dir_file(retpath, datasearchpath[i], filename) == 0)
165                return retpath;
166        }
167   
168        /* we couldn't find the file */
169        return NULL;

** CID 381049:  Memory - illegal accesses  (OVERRUN)
/src/device/rdram/rdram.c: 176 in read_rdram_regs()

________________________________________________________________________________________________________
*** CID 381049:  Memory - illegal accesses  (OVERRUN)
/src/device/rdram/rdram.c: 176 in read_rdram_regs()
170        module = get_module(rdram, address);
171        if (module == RDRAM_MAX_MODULES_COUNT) {
172            *value = 0;
173            return;
174        }
175   
>>>    CID 381049:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array "rdram->regs[module]" of 80 4-byte elements at element index 255 (byte offset 1023) using index "reg" (which evaluates to 255).
176        *value = rdram->regs[module][reg];
177   
178        /* some bits are inverted when read */
179        if (reg == RDRAM_MODE_REG) {
180            *value ^= UINT32_C(0xc0c0c0c0);
181        }

** CID 381048:  High impact quality  (Y2K38_SAFETY)
/src/device/gb/mbc3_rtc.c: 60 in update_rtc_regs()

________________________________________________________________________________________________________
*** CID 381048:  High impact quality  (Y2K38_SAFETY)
/src/device/gb/mbc3_rtc.c: 60 in update_rtc_regs()
54                ++rtc->regs[MBC3_RTC_DAYS_L];
55            }
56            diff /= 24;
57   
58            /* update days counter */
59            unsigned int days = (((rtc->regs[MBC3_RTC_DAYS_H] & 0x01) << 8) | rtc->regs[MBC3_RTC_DAYS_L])
>>>    CID 381048:  High impact quality  (Y2K38_SAFETY)
>>>    A "time_t" value is stored in an integer with too few bits to accommodate it.  The expression "diff" is cast to "int".
60                + (int)diff;
61   
62            rtc->regs[MBC3_RTC_DAYS_L] = days & 0xff;
63            rtc->regs[MBC3_RTC_DAYS_H] = (rtc->regs[MBC3_RTC_DAYS_H] & ~0x01) | (days & 0x100);
64   
65            /* set carry bit if days overflow */

** CID 381047:  Memory - illegal accesses  (OVERRUN)

________________________________________________________________________________________________________
*** CID 381047:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/rdp/rdp_core.c: 109 in write_dpc_regs()
103        case DPC_BUFBUSY_REG:
104        case DPC_PIPEBUSY_REG:
105        case DPC_TMEM_REG:
106            return;
107        }
108   
>>>    CID 381047:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array of 8 4-byte elements at element index 16383 (byte offset 65535) by dereferencing pointer "&dp->dpc_regs[reg]".
109        masked_write(&dp->dpc_regs[reg], value, mask);
110   
111        switch(reg)
112        {
113        case DPC_START_REG:
114            dp->dpc_regs[DPC_CURRENT_REG] = dp->dpc_regs[DPC_START_REG];

** CID 381046:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/rsp/rsp_core.c: 252 in read_rsp_regs()

________________________________________________________________________________________________________
*** CID 381046:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/rsp/rsp_core.c: 252 in read_rsp_regs()
246   
247    void read_rsp_regs(void* opaque, uint32_t address, uint32_t* value)
248    {
249        struct rsp_core* sp = (struct rsp_core*)opaque;
250        uint32_t reg = rsp_reg(address);
251   
>>>    CID 381046:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array "sp->regs" of 8 4-byte elements at element index 16383 (byte offset 65535) using index "reg" (which evaluates to 16383).
252        *value = sp->regs[reg];
253   
254        if (reg == SP_SEMAPHORE_REG)
255        {
256            sp->regs[SP_SEMAPHORE_REG] = 1;
257        }

** CID 381045:  Control flow issues  (DEADCODE)
/subprojects/minizip/zip.c: 1226 in zipOpenNewFileInZip4_64()

________________________________________________________________________________________________________
*** CID 381045:  Control flow issues  (DEADCODE)
/subprojects/minizip/zip.c: 1226 in zipOpenNewFileInZip4_64()
1220   
1221              err = deflateInit2(&zi->ci.stream, level, Z_DEFLATED, windowBits, memLevel, strategy);
1222   
1223              if (err==Z_OK)
1224                  zi->ci.stream_initialised = Z_DEFLATED;
1225            }
>>>    CID 381045:  Control flow issues  (DEADCODE)
>>>    Execution cannot reach this statement: "if (zi->ci.method == 12) {
}".
1226            else if(zi->ci.method == Z_BZIP2ED)
1227            {
1228    #ifdef HAVE_BZIP2
1229                // Init BZip stuff here
1230              zi->ci.bstream.bzalloc = 0;
1231              zi->ci.bstream.bzfree = 0;

** CID 381044:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/mi/mi_controller.c: 86 in read_mi_regs()

________________________________________________________________________________________________________
*** CID 381044:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/mi/mi_controller.c: 86 in read_mi_regs()
80   
81    void read_mi_regs(void* opaque, uint32_t address, uint32_t* value)
82    {
83        struct mi_controller* mi = (struct mi_controller*)opaque;
84        uint32_t reg = mi_reg(address);
85   
>>>    CID 381044:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array "mi->regs" of 4 4-byte elements at element index 16383 (byte offset 65535) using index "reg" (which evaluates to 16383).
86        *value = mi->regs[reg];
87    }
88   
89    void write_mi_regs(void* opaque, uint32_t address, uint32_t value, uint32_t mask)
90    {
91        struct mi_controller* mi = (struct mi_controller*)opaque;

** CID 381043:  Memory - illegal accesses  (OVERRUN)

________________________________________________________________________________________________________
*** CID 381043:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/vi/vi_controller.c: 154 in write_vi_regs()
148        case VI_V_INTR_REG:
149            masked_write(&vi->regs[VI_V_INTR_REG], value, mask);
150            set_vi_vertical_interrupt(vi);
151            return;
152        }
153   
>>>    CID 381043:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array of 14 4-byte elements at element index 16383 (byte offset 65535) by dereferencing pointer "&vi->regs[reg]".
154        masked_write(&vi->regs[reg], value, mask);
155    }
156   
157    void vi_vertical_interrupt_event(void* opaque)
158    {
159        struct vi_controller* vi = (struct vi_controller*)opaque;

** CID 381042:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/vi/vi_controller.c: 108 in read_vi_regs()

________________________________________________________________________________________________________
*** CID 381042:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/vi/vi_controller.c: 108 in read_vi_regs()
102            }
103   
104            /* update current field */
105            vi->regs[VI_CURRENT_REG] = (vi->regs[VI_CURRENT_REG] & (~1)) | vi->field;
106        }
107   
>>>    CID 381042:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array "vi->regs" of 14 4-byte elements at element index 16383 (byte offset 65535) using index "reg" (which evaluates to 16383).
108        *value = vi->regs[reg];
109    }
110   
111    void write_vi_regs(void* opaque, uint32_t address, uint32_t value, uint32_t mask)
112    {
113        struct vi_controller* vi = (struct vi_controller*)opaque;

** CID 381041:  Memory - illegal accesses  (OVERRUN)

________________________________________________________________________________________________________
*** CID 381041:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/rdp/rdp_core.c: 139 in write_dps_regs()
133   
134    void write_dps_regs(void* opaque, uint32_t address, uint32_t value, uint32_t mask)
135    {
136        struct rdp_core* dp = (struct rdp_core*)opaque;
137        uint32_t reg = dps_reg(address);
138   
>>>    CID 381041:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array of 4 4-byte elements at element index 16383 (byte offset 65535) by dereferencing pointer "&dp->dps_regs[reg]".
139        masked_write(&dp->dps_regs[reg], value, mask);
140    }
141   
142    void rdp_interrupt_event(void* opaque)
143    {
144        struct rdp_core* dp = (struct rdp_core*)opaque;
145   
146        raise_rcp_interrupt(dp->mi, MI_INTR_DP);

** CID 381040:  Memory - illegal accesses  (OVERRUN)

________________________________________________________________________________________________________
*** CID 381040:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/ri/ri_controller.c: 52 in write_ri_regs()
46   
47    void write_ri_regs(void* opaque, uint32_t address, uint32_t value, uint32_t mask)
48    {
49        struct ri_controller* ri = (struct ri_controller*)opaque;
50        uint32_t reg = ri_reg(address);
51   
>>>    CID 381040:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array of 8 4-byte elements at element index 16383 (byte offset 65535) by dereferencing pointer "&ri->regs[reg]".
52        masked_write(&ri->regs[reg], value, mask);

** CID 381039:  Memory - illegal accesses  (OVERRUN)

________________________________________________________________________________________________________
*** CID 381039:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/rsp/rsp_core.c: 304 in write_rsp_regs2()
298   
299    void write_rsp_regs2(void* opaque, uint32_t address, uint32_t value, uint32_t mask)
300    {
301        struct rsp_core* sp = (struct rsp_core*)opaque;
302        uint32_t reg = rsp_reg2(address);
303   
>>>    CID 381039:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array of 2 4-byte elements at element index 16383 (byte offset 65535) by dereferencing pointer "&sp->regs2[reg]".
304        masked_write(&sp->regs2[reg], value, mask);
305    }
306   
307    void do_SP_Task(struct rsp_core* sp)
308    {
309        uint32_t save_pc = sp->regs2[SP_PC_REG] & ~0xfff;

** CID 381038:  Memory - illegal accesses  (OVERRUN)

________________________________________________________________________________________________________
*** CID 381038:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/ai/ai_controller.c: 219 in write_ai_regs()
213                ai->samples_format_changed = 1;
214   
215            masked_write(&ai->regs[reg], value, mask);
216            return;
217        }
218   
>>>    CID 381038:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array of 6 4-byte elements at element index 16383 (byte offset 65535) by dereferencing pointer "&ai->regs[reg]".
219        masked_write(&ai->regs[reg], value, mask);
220    }
221   
222    void ai_end_of_dma_event(void* opaque)
223    {
224        struct ai_controller* ai = (struct ai_controller*)opaque;

** CID 381037:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/rsp/rsp_core.c: 296 in read_rsp_regs2()

________________________________________________________________________________________________________
*** CID 381037:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/rsp/rsp_core.c: 296 in read_rsp_regs2()
290   
291    void read_rsp_regs2(void* opaque, uint32_t address, uint32_t* value)
292    {
293        struct rsp_core* sp = (struct rsp_core*)opaque;
294        uint32_t reg = rsp_reg2(address);
295   
>>>    CID 381037:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array "sp->regs2" of 2 4-byte elements at element index 16383 (byte offset 65535) using index "reg" (which evaluates to 16383).
296        *value = sp->regs2[reg];
297    }
298   
299    void write_rsp_regs2(void* opaque, uint32_t address, uint32_t value, uint32_t mask)
300    {
301        struct rsp_core* sp = (struct rsp_core*)opaque;

** CID 381036:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/ai/ai_controller.c: 185 in read_ai_regs()

________________________________________________________________________________________________________
*** CID 381036:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/ai/ai_controller.c: 185 in read_ai_regs()
179                ai->iaout->push_samples(ai->aout, p + diff, ai->last_read - *value);
180                ai->last_read = *value;
181            }
182        }
183        else
184        {
>>>    CID 381036:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array "ai->regs" of 6 4-byte elements at element index 16383 (byte offset 65535) using index "reg" (which evaluates to 16383).
185            *value = ai->regs[reg];
186        }
187    }
188   
189    void write_ai_regs(void* opaque, uint32_t address, uint32_t value, uint32_t mask)
190    {

** CID 381035:  Memory - illegal accesses  (OVERRUN)
/src/device/dd/dd_controller.c: 351 in read_dd_regs()

________________________________________________________________________________________________________
*** CID 381035:  Memory - illegal accesses  (OVERRUN)
/src/device/dd/dd_controller.c: 351 in read_dd_regs()
345            }
346            else {
347                dd->regs[reg] &= ~DD_STATUS_DISK_PRES;
348            }
349        }
350   
>>>    CID 381035:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array "dd->regs" of 19 4-byte elements at element index 63 (byte offset 255) using index "reg" (which evaluates to 63).
351        *value = dd->regs[reg];
352        DebugMessage(M64MSG_VERBOSE, "DD REG: %08X -> %08x", address, *value);
353   
354        /* post read update. Not part of the returned value */
355        switch(reg)
356        {

** CID 381034:  Memory - corruptions  (OVERRUN)
/src/device/dd/dd_controller.c: 530 in write_dd_regs()

________________________________________________________________________________________________________
*** CID 381034:  Memory - corruptions  (OVERRUN)
/src/device/dd/dd_controller.c: 530 in write_dd_regs()
524        case DD_ASIC_CUR_TK: /* fallthrough */
525        case DD_ASIC_CUR_SECTOR:
526            DebugMessage(M64MSG_WARNING, "Trying to write to read-only registers: %08x <- %08x", address, value);
527            break;
528   
529        default:
>>>    CID 381034:  Memory - corruptions  (OVERRUN)
>>>    Overrunning array "dd->regs" of 19 4-byte elements at element index 63 (byte offset 255) using index "reg" (which evaluates to 63).
530            dd->regs[reg] = value;
531        }
532    }
533   
534   
535    void read_dd_rom(void* opaque, uint32_t address, uint32_t* value)

** CID 381033:  Memory - illegal accesses  (OVERRUN)

________________________________________________________________________________________________________
*** CID 381033:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/pi/pi_controller.c: 213 in write_pi_regs()
207        case PI_BSD_DOM2_PGS_REG:
208        case PI_BSD_DOM2_RLS_REG:
209            masked_write(&pi->regs[reg], value & 0xff, mask);
210            return;
211        }
212   
>>>    CID 381033:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array of 13 4-byte elements at element index 16383 (byte offset 65535) by dereferencing pointer "&pi->regs[reg]".
213        masked_write(&pi->regs[reg], value, mask);
214    }
215   
216    void pi_end_of_dma_event(void* opaque)
217    {
218        struct pi_controller* pi = (struct pi_controller*)opaque;

** CID 381032:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/pi/pi_controller.c: 156 in read_pi_regs()

________________________________________________________________________________________________________
*** CID 381032:  Memory - illegal accesses  (OVERRUN)
/src/device/rcp/pi/pi_controller.c: 156 in read_pi_regs()
150   
151    void read_pi_regs(void* opaque, uint32_t address, uint32_t* value)
152    {
153        struct pi_controller* pi = (struct pi_controller*)opaque;
154        uint32_t reg = pi_reg(address);
155   
>>>    CID 381032:  Memory - illegal accesses  (OVERRUN)
>>>    Overrunning array "pi->regs" of 13 4-byte elements at element index 16383 (byte offset 65535) using index "reg" (which evaluates to 16383).
156        *value = pi->regs[reg];
157   
158        if (reg == PI_WR_LEN_REG || reg == PI_RD_LEN_REG)
159            *value = 0x7F;
160        else if (reg == PI_CART_ADDR_REG)
161            *value &= 0xFFFFFFFE;
richard42 commented 2 years ago

I kind of think these coverity scans are mostly bogus. I don't think any of this is new code - it just periodically runs a static analysis on the code and spits out a few warning messages purely as a sales/marketing exercise. If "reg" was ever set to 16,383 we would have real problems. Some of those might point to places in the code where we could improve security by using snprintf() instead of sprintf(), but it's probably not super important

Narann commented 2 years ago

Thanks, closing the issue.