Bug #96143 Error executing stored routines
Submitted: 9 Jul 2019 15:45 Modified: 10 Mar 2020 17:32
Reporter: Martin Dunsmore Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / NET Severity:S2 (Serious)
Version:8.0.16 OS:Any
Assigned to: CPU Architecture:Any

[9 Jul 2019 15:45] Martin Dunsmore
Description:
We have a lot of MySQL users that make use of stored routines that are defined with security invoker. Since MySQL has abandoned the mysql.proc table, for some unknown reason the .NET Connector is executing "SHOW CREATE PROCEDURE ..." to get parameters. This is all well and good if we were okay giving out MySQL users global select permissions, which by the way, we're not!

How to repeat:
Create some MySQL objects:

CREATE DATABASE IF NOT EXISTS aaaaa
CHARACTER SET utf8
COLLATE utf8_general_ci;

USE aaaaa;

CREATE TABLE IF NOT EXISTS hello (
  id int(11) NOT NULL AUTO_INCREMENT,
  string varchar(255) DEFAULT NULL,
  PRIMARY KEY (id)
)
ENGINE = INNODB,
CHARACTER SET utf8,
COLLATE utf8_general_ci;

DELIMITER $$

CREATE PROCEDURE get_hello(
  IN p_id int
  )
SQL SECURITY INVOKER
BEGIN
  SELECT
    *
  FROM hello
  WHERE id = p_id;
END
$$

DELIMITER ;

INSERT INTO hello (string) VALUES ('Hello World!');

CREATE USER 'atest'@'localhost' IDENTIFIED BY 'PICK-A-PASSWORD' PASSWORD EXPIRE NEVER;
GRANT SELECT ON TABLE aaaaa.hello TO 'atest'@'localhost';
GRANT EXECUTE ON PROCEDURE aaaaa.get_hello TO 'atest'@'localhost';

Create a new .NET console app (I used .NET Core 2.2) and drop in the following code:

using (MySqlConnection c = new MySqlConnection("server=localhost;port=3306;uid=atest;pwd=nEm1s1s69;database=aaaaa"))
{
	c.Open();

	MySqlCommand cmd = new MySqlCommand();
	cmd.Connection = c;

	cmd.CommandText = "get_hello";
	cmd.CommandType = CommandType.StoredProcedure;

	cmd.Parameters.AddWithValue("p_id", 1);

	MySqlDataAdapter da = new MySqlDataAdapter();
	da.SelectCommand = cmd;

	DataSet ds = new DataSet();
	da.Fill(ds);

	if (ds.Tables.Count > 0)
	{
		Console.WriteLine($"Result: {ds.Tables[0].Rows[0]["col_b"].ToString()}");
	}
}

It will throw an exception: System.Data.SqlTypes.SqlNullValueException: 'Data is Null. This method or property cannot be called on Null values.'

Now execute: GRANT SELECT ON *.* TO 'atest'@'localhost';

Re-run the console app and it will work, but now the user has read access to all your, possibly very sensitive tables.

Suggested fix:
Use information_schema to get stored routine parameters
[10 Jul 2019 7:07] MySQL Verification Team
Hello Martin,

Thank you for the report and test case.

regards,
Umesh
[10 Jul 2019 14:15] Bradley Grainger
For Connector/NET, you can work around this by adding ";CheckParameters=false" to your connection string. However, this will stop Connector/NET from ensuring that the parameters used in your C# code are the same as the parameters defined for the stored procedure; you will have to manually ensure that all parameters are the same type and in the right order.

Alternatively, you can switch to MySqlConnector https://mysqlconnector.net/ that does use INFORMATION_SCHEMA (if mysql.proc isn't available) for stored procedure metadata.
[13 Jul 2019 15:33] Bradley Grainger
This may be a duplicate of bug #75301.
[5 Sep 2019 20:08] Mats Gefvert
Just came across this in the latest Connector/NET and MySQL Community Server, both version 8.0.17. Same thing as above, if I run ExecuteNonQuery() with a CommandType.StoredProcedure it fails, but when I run a direct "call ..." query it works. The calling user is restricted to a single database; if I set mysql.user.Select_Priv='Y' it works beautifully.

The trouble seems to be in ISSchemaProvider.cs, line 195-206.

      try
      {
        if (connection.Settings.HasProcAccess)
          return base.GetProcedures(restrictions);
      }
      catch (MySqlException ex)
      {
        if (ex.Number == (int)MySqlErrorCode.TableAccessDenied)
          connection.Settings.HasProcAccess = false;
        else
          throw;
      }

The code is supposed to mitigate not having access to the INFORMATION_SCHEMA.ROUTINES table, but it receives an exception it did not plan on.

The offending line (for me at least) is in SchemaProvider.cs, line 644,

      row["ROUTINE_DEFINITION"] = reader.GetString("routine_definition");

which is NULL instead of containing a value. So it throws a System.Data.SqlTypes.SqlNullValueException which clearly does not inherit from MySqlException; it's never caught by ISSchemaProvider, and thus propagates out to the caller.

The fix seems easy; either check for the missing routine definition, or just catch SqlNullValueException and turn HasProcAccess off.
[6 Sep 2019 7:47] Martin Dunsmore
We made some changes to the connector, the issue is the connector wanting the body of the routine when it doesn't need the body of the routine. All it cares about the parameter order so it can be helpful and re-order your parameters if you supplied them in the wrong order. Best solution is to turn this of, it save some round trips to the database but that wasn't an option for us so we made the following changes:

ISSchemaProvider.cs:
Method: GetProcedureParameters

Inside the try, detect the server version and always use information schema
if (connection.driver.Version.isAtLeast(8, 0, 1))
{
   return GetParametersFromIS(restrictions, routines);
}

As has been noted already, inside SchemaProvider.cs, you need to handle the NULL routine definition.
row["ROUTINE_DEFINITION"] = reader["routine_definition"] == DBNull.Value ? "" : reader.GetString("routine_definition");

The fact that GLOBAL SELECT is required for something like this without custom changes to the connector is incredibly lazy or a massive oversight.
[9 Mar 2020 23:43] Gustavo Cuatepotzo
Posted by developer:
 
The method reader.GetString was raising the null exception when retrieving ROUTINE_DEFINITION, it is modified to handle the null value,

ROUTINE_DEFINITION is show as null if user is not the creator of the routine, is possible grant the privilege SHOW_ROUTINE
[10 Mar 2020 17:32] Christine Cole
Posted by developer:
 
Fixed as of the upcoming MySQL Connector/NET 8.0.21 release, and here's the proposed changelog entry from the documentation team:

Attempts to execute a function or stored procedure returned a null
exception unexpectedly when the caller was not the creator of the routine.
This fix introduces a mechanism to manage null values for these cases,
permits the granting of privilege to SHOW_ROUTINE, and revises
SqlNullValueException to identify when a user account lacks adequate
permission to access a routine.

Thank you for the bug report.