2

We have a use case where an app that sends out emails finds a specific string ('smart tag') in an email and replaces it with the results of a stored procedure.

So for example the email could have Dear <ST:Name> in the body, and then the code would identify this string, run the stored procedure to find the client name passing in the client id as a parameter.

The list of these tags and the stored procedures that need to be run are currently hard coded, so every time a new 'smart tag' needs to be added, a code change and deployment is required.

Our BA's our skilled in SQL and want to be able to add new tags manually.

Is it bad practice to store the procedure and parameters in a database table? Would this be a suitable design for such a table? Would it be necessary to store parameter type?

SmartTag 

SmartTagId   SmartTag   StoredProcedure

SmartTagParameters 

SmartTagParameterId   SmartTagId   ParameterName
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
woggles
  • 7,444
  • 12
  • 70
  • 130
  • I would caution against potential [SQL Injection](http://en.wikipedia.org/wiki/SQL_injection) attacks with this approach. Although, you could quite easily add some simple checks to strip out anything that seemed suspect from the values before processing the data. Regarding your question about storing the data type, I assume you would be casting everything to a string/varchar data type anyways, so it probably wouldn't be necessary. However, it may be useful if you're going to have a front-end that could display a different editor control based upon the data type (e.g. a date picker, numeric tex – Alexander Nov 06 '12 at 08:59

3 Answers3

4

Table driven configuration, data driven programming, is good.

The primary thing to watch out for is SQL Injection risk (or in your case it would be called 'tag injection'...): one could use the email as an attack vector to gain elevated privileges by inserting a crafted procedure that would be run under higher privileges. Note that this is more than just the usuall caution around SQL Injection, since you are already accepting arbitrary code to be executed. This is more of a sandboxing problem.

Typical problems are from the type system: parameters have various types but the declaration tables have a string type for them. SQL_VARIANT can help.

Another potential problem is the language to declare and discover tags. Soon you'll be asked to recognize <tag:foo>, but only before <tag:bar>. A fully fledged context sensitive parser usually follows shortly after first iteration... It would be helpful if you can help by leveraging something already familiar (eg. think how JQuery uses the CSS selector syntax). HTMLAgilityPack could help you perhaps (and btw, this is a task perfect for SQLCLR, don't try to build elaborate statefull parser in T-SQL...).

Remus Rusanu
  • 288,378
  • 40
  • 442
  • 569
  • +1 for mentioning use of `SQL_VARIANT` data type & SQL Injection. – Alexander Nov 06 '12 at 09:06
  • Its not going to be possible to add tags through the front end - only BAs will be able to add them using management studio. So I think that should remove any worry of SQL injection?The primary reason for making the change would be to avoid a deployment when new tags need to be added. – woggles Nov 06 '12 at 09:09
1

It's not bad practice, what you are doing is totally fine. As long as only your admin/BA can add and change parameters and change configuration you do not have to worry about injection. If users can add and change parameters you really need to check there input and whitelist certain chars.

It's not only sql injection you have to check, but cross site scripting and dom injection and cross site request forgery as well. The merged text is displayed on a users computer, so you have to protect him when viewing your merge result.

Peter
  • 27,590
  • 8
  • 64
  • 84
0

Interesting question, will follow up as it has something to do with mine. Is not bad practice at all. In fact, we are using same approach. I'm trying to achieve similar goals on my XSL editor. I'm using a combination of XML tags, stored procedures and VB.Net logic to do the replacements.

I'm using a combination of a table with all the used XML tags (they are used on other places on the application) and stored procedures that do all the dirty job. One set of sp transforms from text with tags to a user readable text. Other set of procedures creates an XML tree from the XML tags table so the users can choose from to edit their text.

SQL Injection is not an issue for us as we use these procedures to create emails, not to parse them from external sources.

Regarding a comment on one of the question, we manage the tags also directly from SSMS, no admin window to manage them, at least for now. But we plan to add a simple admin window to manage the tags so it would be easier to add/delete/modify them once the application is deployed.

Community
  • 1
  • 1
Yaroslav
  • 6,476
  • 10
  • 48
  • 89