views:

2095

answers:

5

I am writing a web server application in C# and using StreamReader class to read from an underlying NetworkStream:

 NetworkStream ns = new NetworkStream(clientSocket);
 StreamReader sr = new StreamReader(ns);
 String request = sr.ReadLine();

This code is prone to DoS attacks because if the attacker never disconnects we will never finish reading the line. Is there any way to limit the number of characters read by StreamReader.ReadLine() in .NET?

+4  A: 

You would have to use the Read(char[], int, int) overload (which does limit the length) and do your own end-of-line detection; shouldn't be too tricky.

For a slightly lazy version (that uses the single-characted reading version):

static IEnumerable<string> ReadLines(string path, int maxLineLength)
{
    StringBuilder currentLine = new StringBuilder(maxLineLength);
    using (var reader = File.OpenText(path))
    {
        int i;
        while((i = reader.Read()) > 0) {
            char c = (char) i;
            if(c == '\r' || c == '\n') {
                yield return currentLine.ToString();
                currentLine.Length = 0;
                continue;
            }
            currentLine.Append((char)c);
            if (currentLine.Length > maxLineLength)
            {
                throw new InvalidOperationException("Max length exceeded");
            }
        }
        if (currentLine.Length > 0)
        {
            yield return currentLine.ToString();
        }                
    }
}
Marc Gravell
A: 

You can always use ".Read(...)" and MSDN recommends doing so for a situation like yours.

http://msdn.microsoft.com/en-us/library/system.io.streamreader.readline.aspx

Austin Salonen
+1  A: 

You might need one of StreamReader.Read overload:

Taken from http://msdn.microsoft.com/en-us/library/9kstw824.aspx

    using (StreamReader sr = new StreamReader(path)) 
    {
        //This is an arbitrary size for this example.
        char[] c = null;

        while (sr.Peek() >= 0) 
        {
            c = new char[5];
            sr.Read(c, 0, c.Length);
            //The output will look odd, because
            //only five characters are read at a time.
            Console.WriteLine(c);
        }
    }

Focus on the sr.Read(c, 0, c.Length) line. This reads only 5 character from stream and put in into c array. You may want to change 5 to value you want.

m3rLinEz
A: 

Here is my own solution based on the solution by Marc Gravell:

using System;
using System.IO;
using System.Text;

namespace MyProject
{
    class StreamReaderExt : StreamReader
    {

        public StreamReaderExt(Stream s, Encoding e) : base(s, e)
        {            
        }

        /// <summary>
        /// Reads a line of characters terminated by CR+LF from the current stream and returns the data as a string
        /// </summary>
        /// <param name="maxLineLength">Maximum allowed line length</param>
        /// <exception cref="System.IO.IOException" />
        /// <exception cref="System.InvalidOperationException">When string read by this method exceeds the maximum allowed line length</exception>
        /// <returns></returns>
        public string ReadLineCRLF(int maxLineLength)
        {
            StringBuilder currentLine = new StringBuilder(maxLineLength);

            int i;
            bool foundCR = false;
            bool readData = false;

            while ((i = Read()) > 0)
            {

                readData = true;

                char c = (char)i;

                if (foundCR)
                {
                    if (c == '\r')
                    {
                        // If CR was found before , and the next character is also CR,
                        // adding previously skipped CR to the result string
                        currentLine.Append('\r');
                        continue;
                    }
                    else if (c == '\n')
                    {
                        // LF found, finished reading the string
                        return currentLine.ToString();
                    }
                    else
                    {
                        // If CR was found before , but the next character is not LF,
                        // adding previously skipped CR to the result string
                        currentLine.Append('\r');
                        foundCR = false;
                    }
                }
                else // CR not found
                {
                    if (c == '\r')
                    {
                        foundCR = true;
                        continue;
                    }
                }

                currentLine.Append((char)c);
                if (currentLine.Length > maxLineLength)
                {
                    throw new InvalidOperationException("Max line length exceeded");
                }
            }

            if (foundCR)
            {
                // If CR was found before, and the end of the stream has been reached, appending the skipped CR character
                currentLine.Append('\r');
            }

            if (readData)
            {
                return currentLine.ToString();
            }

            // End of the stream reached
            return null;

        }
    }
}

This piece of code is provided "AS IS" with NO WARRANTY.

Roman Shumikhin
A: 

So here's a question... Assuming you have complete control over the part of the code doing the reading (StreamReader.ReadLine()) and the file from which it is reading, is there any reason to worry about a DoS attack? Let's say this functionality is on an employee's workstation (laptop) and the employee has the ability(not the need) to modify the file being read. Is this still a concern? At some point, you have to place a certain level of trust in your employees. If you can't trust them, who can you trust. Yes, I'm aware of rogue employees, but from a configuration file standpoint, is there really any worry? In addition, if the employee did tinker with the file being read to cause the program to get DoS'ed, it would really only affect his/her instance of the installed software, preventing him/her from being productive.

Thoughts?

mark