I have been dealing with code audits, security analysis like this one and ethical hacking runs for a decade and a half now, so let me share some of my experience.
Every single boss and team lead I've had that I saw as a role model insisted on having high standards for code quality, not just to make code reviews easier but also in name of stability, maintainability and security. Devs who do exception handling like your team either have little experience, have little time or simply don't care. And this kind of lazy coding can lead to disaster.
The most glaring one to my eyes is your problem number 3, an empty catch. That masks whatever error passes through it, not even logging it. Now suppose the following happens:
try {
// start a transaction
} catch (Exception ex) {
}
finally {
// commit the transaction
}
You could be putting data into the database that is invalid for the business. For a more elaborate example, suppose a type Account with a property funds, such that funds is an unsigned integer (because you don't want to allow an account with less than zero money in it):
try {
// moving funds between accounts
Transaction.start();
accountA.funds += amount;
accountB.funds -= amount; // throws an overflow exception if funds < amount
} catch (Exception ex) {
}
finally {
Transaction.commit();
}
The end result can be money going to account A, but not being subtracted from account B.
This may sound stupid and beneath your team for you, but I've had to deal with such things in legacy code at some companies I've worked for. I'm not saying that your solution has this kind of code in it, but I am saying that if you have lazy or disgruntled staff don't be surprised to find code like that. And never trust your codebase to be fully secure if you are getting a report like the one you did.
Let me go one step ahead and give you an example in which bad error handling really soured the day for a lot of people. One of the most expensive coding errors in history was that of the Ariane 5 rocket's first test flight.
Ariane 5's first test flight (Ariane 5 Flight 501) on 4 June 1996 failed, with the rocket self-destructing 37 seconds after launch because of a malfunction in the control software. A data conversion from 64-bit floating point value to 16-bit signed integer value to be stored in a variable representing horizontal bias caused a processor trap (operand error) because the floating point value was too large to be represented by a 16-bit signed integer. The software was originally written for the Ariane 4 where efficiency considerations (the computer running the software had an 80% maximum workload requirement) led to four variables being protected with a handler while three others, including the horizontal bias variable, were left unprotected because it was thought that they were "physically limited or that there was a large margin of safety".
Emphasis mine. If I remember correctly that was a 370 million dollars failure. Granted, you probably aren't coding for NASA or DARPA, so your losses due to bad coding should be mostly staff time due to rework rather than exploding machinery. But losses are losses just the same, and if you can avoid them by being just a bit more careful you'd better at least consider it.
