• Registered: 2010-10-14
  • Posts: 5

Topic: Serious security flaws...

I'm new to the punBB arena, however I'm a professional .Net developer for an agency of the federal government.  While looking at the code, there seems to be a lot of overhead and some security flaws.  First of all, NEVER EVER EVER put your Select, Update, Insert, and Delete statements in the code (especially the HTML).  The best way to accomplish this is to create scripts during the installation process that will create stored proceedures.  All the database formats that you are trying to support, support SPs and Macros.
(Not that anyone owes me an answer, but something to think about) - Why would you extend a control in the APP_CODE folder?  This creates an enormous amount of overhead for the application (as opposed to simply creating a custom server control extension).  The last thing and most important is encrypting the connection string (for obvious reasons).  This is extremely easy to do in code-behind.  I typically give a button event to encrypt and decrypt the string. 
I understand it is just a beta release, just trying to give ya'all some food for thought.

  • Registered: 2010-10-14
  • Posts: 5

Re: Serious security flaws...

Also,
Whoever did the Themes support really doesn't know how to use it.  You create a page for the controls and use CSS for the rest and use the page declarative (or web.config) to define which theme you are wanting to use.

There are really some fundamental best practices that are completely missing from the .NET project.  You should use a basepage for inheritance and build from there.  This is (and is not) happening.  If you use your basepage as the security module, you won't have to get your connection string but once a page (thus limiting the security exposure) and setup your connection.  This is also a good place to start your business logic (as opposed to the data or presentation).

I hope I don't seem to be 'hating' on the project too much.  Just trying to offer some constructive criticism and food for thought.  I'm actually impressed with the provider work that was done, however some of your override methods are not being triggered due to referencing errors.

Hit me up if you have questions or need examples...

  • Registered: 2010-10-14
  • Posts: 5

Re: Serious security flaws...

cgcarter1 wrote:

The last thing and most important is encrypting the connection string (for obvious reasons).  This is extremely easy to do in code-behind.  I typically give a button event to encrypt and decrypt the string. 
I understand it is just a beta release, just trying to give ya'all some food for thought.

Here is some quick code that you can drop into a class library to encrypt/decrypt the connection string in the web.config...

Public Function EncryptConfig() As Boolean
            Try
                Dim confg As Configuration = WebConfigurationManager.OpenWebConfiguration("")
                Dim confStrSect As ConfigurationSection = confg.GetSection(section)
                If Not confStrSect Is Nothing Then
                    confStrSect.SectionInformation.ProtectSection(provider)
                    confg.Save()
                End If
                ' the encrypted section is automatically decrypted!!
                Return True
            Catch ex As Exception
                Return False
            End Try
        End Function

        Public Function DecryptConfig() As Boolean
            Try
                Dim confg As Configuration = WebConfigurationManager.OpenWebConfiguration("")
                Dim confStrSect As ConfigurationSection = confg.GetSection(section)
                If Not confStrSect Is Nothing AndAlso confStrSect.SectionInformation.IsProtected Then
                    confStrSect.SectionInformation.UnprotectSection()
                    confg.Save()
                End If
                Return True
            Catch ex As Exception
                Return False
            End Try

        End Function
  • Registered: 2010-10-14
  • Posts: 5

Re: Serious security flaws...

There is an error in the Membership Provider.  On the OdbcCommand it calls for a uniqueidentifier for the PKID field.  The field in the table is called UserId and a varchar.  This throws an exception.  The fieldname in the insert command should be UserId and the type in the cmd.Parameters.AddWithValue should be providerUserKey.ToString and remove the uniqueidentifier.

  • Registered: 2010-10-14
  • Posts: 5

Re: Serious security flaws...

Also there is a typo in the ODBC queries.  It looks for a column LastPasswordChangedDate when the column in the Users table it LastPasswordChangeDate.  I am rewriting the module into a class library for providers.  Note this is only going to support SQL Server but if you want the code, just hit me up.

  • Gene53
  • Junior Member
  • Offline
  • From: Canada
  • Registered: 2010-11-28
  • Posts: 7

Re: Serious security flaws...

Very interesting thread, I wonder why a developer hasn't replied to it or better yet, invited you to join the core development team.

Cheers,
Gene

  • tomjol
  • Junior Member
  • Offline
  • Registered: 2007-10-17
  • Posts: 2

Re: Serious security flaws...

Quite simple really, Gene53 - I suspect there isn't really a development team at all. Considering the code was forked into FluxBB when Informer bought PunBB from Rickard and all the updates to PunBB since then appear to be merges from FluxBB's codebase, it's unlikely Informer have much development power allocated to PunBB whatsoever. In addition, this thread and another highlight that the .NET version is clearly being written by someone severely lacking in .NET experience and now appears largely abandoned - most likely, it's a hobby project, and whoever was writing it got bored.

  • Registered: 2009-06-02
  • Posts: 23

Re: Serious security flaws...

Yup ... I  believed the developer thought that .NET is same as PHP so most of the things are looking exactly like PHP.

http://www.vnfox.com
  • Registered: 2013-08-06
  • Posts: 3

Re: Serious security flaws...

I hope these security flaws will be wiped away in the next release of punbb. Im new to using punbb but im liking it smile

10 Reply by Trace

  • Trace
  • Trace
  • PunBB senior member
  • Offline
  • Registered: 2012-07-01
  • Posts: 560

Re: Serious security flaws...

It's regarding PunBB.NET, another project, separated from PunBB, so don't worry about security issues smile.