0

Since C# doesn't provide a mechanism for friend functions and classes, internal keyword doesn't help if I don't distribute code in separate assemblies and I can't always use nested classes, I came up with:

public sealed class FriendlyClass
{
private readonly string SecretKey = "MySecretKey";
public string PublicData { get; set; }
private string privateData;

public FriendlyClass()
{
    PublicData = "My public data";
    privateData = "My Private Data";
}

public string GetPrivateData(string key = null)
{
    if (key == SecretKey)
        return privateData;
    else throw new System.InvalidOperationException("You can't call this method directly.");

    return null;
}

public void  SetPrivateData(string newData, string key = null)
{
    if (key == SecretKey)
        privateData = newData;
    else throw new System.InvalidOperationException("You can't call this method directly.");
}
}

FriendlyClass friend = new FriendlyClass();

public void FriendlyMethod(string data)
{
    friend.SetPrivateData(data, "MySecretKey");
}

public void StrangerMethod(string data)
{
    friend.SetPrivateData(data);
}

FriendlyMethod("Data is changed");
StrangerMethod("Data is not changed");

It's ugly and lots to write. My question is: can someone still abuse the public methods of FriendlyClass, crash the code or produce unexpected behaviour?

Vlad Radu
  • 337
  • 1
  • 13
  • Not answer-worthy, but in short: Yes. Are you familiar with Reflection? – Josh E Oct 30 '18 at 16:07
  • 1
    What are you actually trying to do? And of course people can mess around and break things with reflection if they just really want to break their own program – harold Oct 30 '18 at 16:07
  • 1
    There is apparently a brother of "Friend", via a atribute: https://stackoverflow.com/a/204744/3346583 – Christopher Oct 30 '18 at 16:07
  • 1
    The `friend` mechanism from other languages isn't designed as a security mechanism to prevent a malicious user executing code using your library from calling those mechanisms. It's just a indicator to callers that the method in question wasn't designed for their use, and that they shouldn't concern themselves with it. This involves compile time support, so that they would have to go around the compiler to do it, but they certainly could. Given that you're only checking it at runtime anyway, you're not accomplishing any of the benefits of the feature you're trying to emulate. – Servy Oct 30 '18 at 16:09
  • "Since C# doesn't provide a mechanism for friend functions and classes" That´s not true, there´s the `internalsVisibleTo`-attribute for exactly this purpose. – MakePeaceGreatAgain Oct 30 '18 at 16:14
  • I am not talking about reflection, just preventing direct usage of some methods or accesing some data. – Vlad Radu Oct 30 '18 at 16:25
  • If you arent distributing code in separate assemblies that implies to me that other developers have access to this code, so whats to stop them just looking at the method/class and finding out what the 'secret key' is. I'll be blunt with you, this is an all round bad idea and just points to poor object design – Dave Oct 30 '18 at 16:27
  • @Christopher, HimBromBeere I already said in the question: "internal keyword doesn't help if I don't distribute code in separate assemblies". The said atribute is about using the internal keword and distributing your code in a separate assembly. – Vlad Radu Oct 30 '18 at 16:29
  • @Dave, if they have acces to the source files, they can also change the access modifiers. I am not trying to hide something, I am trying to impose a policy on usage. – Vlad Radu Oct 30 '18 at 16:30
  • But it's not really imposing anything - especially if you distribute the source files. People can and they will peek the "secret" code. So either something should be settable from outside (and in this case it probably should) or it shouldn't. If it should - make it public, if it shouldn't - make it private or internal. – Szab Oct 30 '18 at 16:40

1 Answers1

1

Access modifiers are not supposed to guarantee data safety, but rather ensure code quality and restrict ways of calling some methods or accessing some data. But even if you create a class where all fields and methods are marked as private, there are ways which allow programmers to access these members from "unauthorized" places in the code. Reflection can be considered to be one of these methods. Take this code for example:

public sealed class Example
{
    private string field = "secure";

    private void PrintField()
    {
        Console.WriteLine(this.field);
    }
}

public class Program
{
    public static void Main()
    {
        var example = new Example();
        var field = example.GetType().GetField("field", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
        var method = example.GetType().GetMethod("PrintField", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);

        field.SetValue(example, "hacked");
        method.Invoke(example, new object[] { });
        Console.ReadLine();
    }
}

Even though both the method and field in the Example class are marked as private, a caller from the Program class was able to overrule that and both set a value in the field and call the method.

So my answer is - yes, somebody can still abuse that by using Reflection, direct memory operations etc. If somebody wants to "hack" your code - they will do it anyway. There is no reason to make the life harder for those who doesn't. In my opinion using pseudofriendly classes like that give a programmer a false sense of security at a high cost of reduced code clarity and unnecessary operations which are required to use the class.

Szab
  • 1,263
  • 8
  • 18
  • What to do? If you want to produce a public API, users not knowing the internals of that API can missuse it and produce a lot of bugs if too many methods are public. They see it in intellisense and use it. Some thing being "private" not only hides it in intellisense, but tells the users the said method is not for public consumption. – Vlad Radu Oct 30 '18 at 16:41
  • But the thing is that if a method is not for public consumption, then why would you even bother creating ways to "securily" use it? And if it is for public consumption after all but you worry that somebody can provide some incorrect input that will cause some bugs - make sure that this method is well documented and provide some validation. – Szab Oct 30 '18 at 16:46
  • Also. Your public API is to be used in code and your user base is programmers - by definition they have a general knowledge about what they do. They are not some mindless monkeys who will click whatever they see and then push it to production. And if they are, it's not really your problem but it's a problem of the company which employs them and their policies :) – Szab Oct 30 '18 at 16:56
  • "why would you even bother creating ways to "securily" use it" The method should be used from another class. The only way to do it in C#, beside using nested classes it's to make it public. – Vlad Radu Oct 30 '18 at 16:59
  • No offence, but problems like that strongly indicate that your solution is not well designed and it lacks some properly defined interfaces and responsibilities of each class. If a method should be used just by one class - why won't you create it in that class? If other classes should be able to use it as well - why not make it public? – Szab Oct 30 '18 at 17:03
  • The method might be used 99% inside the class it's defined. There might be 1% use cases like extension methods or operator overloading when another method needs access and you can't define that method in the class. – Vlad Radu Oct 30 '18 at 17:41