Re: Reduce large amount of logicals in SUBFL pgm, take in another direction

Gary,

I suspect this is where Charles's clause about "probably more work than a
hacker can do" (or something like that) comes in. I agree that if you have a
field like this where you have designed your interface such that the bit
that the user can input is small (so less likely for them to be able to
insert something duplicitous) *and* where you can be sure that the only way
to access this program is through the green-screen (which may further limit
the values that they could enter), then your application is safe.

However...

If someone takes your code and uses that as the basis for a web front-end to
the existing back-end processing, you could be screwed.

If the field size is large enough (it doesn't take many to enter e.g. "";drop
CustomerFile;), you could be screwed.

If some junior programmer who doesn't know SQL as well as you copies this
method, but isn't aware of the potential pitfalls, you could be screwed.

If someone enters weirdly formatted SQL data, you could be screwed (because
your application might crash, even if your database isn't directly affected.

By using parameters, you avoid *all* of these problems. It's no harder than
using a concatenated string. So why not use it?

Rory

On Mon, Aug 1, 2011 at 12:37 PM, Monnier, Gary <Gary.Monnier@xxxxxxxxx>wrote:

Rory,

If, as in my example, you provide fields for WHERE and ORDER BY to a piece
of code that concatenates them to be used in a SELECT statement you are no
more vulnerable to SQL injection attacks than you are having someone gain
access to the system and use SQL to directly access your physical files.

Gary Monnier
IT Software Engineer II
T 425.895.6457
F 425.556.8777
E Gary.Monnier@xxxxxxxxx

Terex Aerial Work Platforms
18465 NE 68th St.
Redmond, WA 98052
www.genielift.com


-----Original Message-----
From: rpg400-l-bounces@xxxxxxxxxxxx [mailto:rpg400-l-bounces@xxxxxxxxxxxx]
On Behalf Of Rory Hewitt
Sent: Monday, August 01, 2011 12:17 PM
To: RPG programming on the IBM i / System i
Subject: Re: Reduce large amount of logicals in SUBFL pgm,take in another
direction

Gary,

I have been known to disagree with Charles on assorted matters, but on this
one, he's absolutely correct.

If you have code somewhere in your application which directly concatenates
strings to form an SQL statement and one of those strings comes directly
from the user, you may be at risk of SQL injection (depending on the
specific strings). It doesn't matter whether this is in a top-level program
or some code in a module that's bound into a service program way deep in
your application.

As has been pointed out, you can mitigate/avoid this, by either using
parameters or by manually 'cleaning' the user-provided string yourself.

Rory

On Mon, Aug 1, 2011 at 11:25 AM, Monnier, Gary <Gary.Monnier@xxxxxxxxx

wrote:

Charles,

So we choose to disagree.

Gary


-----Original Message-----
From: rpg400-l-bounces+gary.monnier=terex.com@xxxxxxxxxxxx [mailto:
rpg400-l-bounces+gary.monnier=terex.com@xxxxxxxxxxxx] On Behalf Of
rpg400-l-bounces+Charles
Wilt
Sent: Monday, August 01, 2011 11:19 AM
To: RPG programming on the IBM i / System i
Subject: Re: Reduce large amount of logicals in SUBFL pgm,take in
another direction

Gary,

You seem to be hung up on *MODULE...

I repeat, it doesn't matter where the statement is built. If you're
building a statement dynamically with concatenation of user supplied

string.

The code is vulnerable. It doesn't have anything to do with *PGM,
*MODULE, *SRVPGM, authority or debug mode.

Charles

On Mon, Aug 1, 2011 at 1:54 PM, Monnier, Gary <Gary.Monnier@xxxxxxxxx>
wrote:

Charles,

With respect you seem to be hung up on my example being a *PGM. I

recommended *MODULE. If you feel someone can get to a *PGM build from
*MODULEs and can inject an SQL statement then again I assert security
settings to the program are set incorrectly. Either you allow *PUBLIC
to debug the program or you have had a security breach involving a
profile with enough authority to debug the program. If the executable
*PGM allows modules and or service programs to be changed then you
also have a security hole. In both cases it is not my example causing
the breach and other much larger security issues are in play.

Respectfully,

Gary Monnier


-----Original Message-----
From: rpg400-l-bounces@xxxxxxxxxxxx
[mailto:rpg400-l-bounces@xxxxxxxxxxxx] On Behalf Of Charles Wilt
Sent: Monday, August 01, 2011 6:06 AM
To: RPG programming on the IBM i / System i
Subject: Re: Reduce large amount of logicals in SUBFL pgm,take in
another direction

Gary,

If you think that your program doesn't open you up to SQL injection,
you

are sadly mistaken...

The fact that the procedure is bound statically by reference or by
copy

as opposed to a dynamic called *PGM has no bearing.

SQL injection attacks can occur anytime you dynamically build a
statement

and concatenate user string input into the statement. It doesn't
matter where the statement is built.

Again, all that matters is that in some way shape or form, you are

building a statement like so:

wSqlStmt = 'SELECT * FROM MYTABLE WHERE MYFLD = ' +
userEnteredValue;

Theoretically, you could parse and sanitize the user input, making
sure it's safe before using it. However, unlike some languages RPG
doesn't have that functionality built in. Also, IMHO you're
dependent on the guy who wrote the sanitizer being smarter than the
hackers :)

The safe way to do dynamic statements is via parameter markers. You

build a statement like so:

wSqlStmt = 'SELECT * FROM MYTABLE WHERE MYFLD = ?';

then when you open/execute it, you pass in the variables to use at
the

parameter markers.

open C1 using :userEnteredValue;

With respect to PCI requirements...
6.5 Develop applications based on secure coding guidelines and
review

custom application code to identify coding vulnerabilities. Follow
up-to-date industry best practices to identify and manage

vulnerabilities.

OWASP is one of the standards usually used for secure coding...their
top

security risk for 2010...injection (SQL queries, LDAP queries, XPath
queries, OS commands, program arguments, etc. ).

https://www.owasp.org/index.php/Category:OWASP_Top_Ten_Project

Thus, dynamic SQL as used by your programs would fail an secure code

review and you'd likely fail a PCI audit.

HTH,
Charles






On Fri, Jul 29, 2011 at 2:26 PM, Monnier, Gary
<Gary.Monnier@xxxxxxxxx>

wrote:

Your opinion Charles and what seems a very strong opinion. J



Yes, I've heard of SQL Injection attacks and no my suggestion does
not leave you open to this sort of attack. I've also heard of LDAP
injection attacks.



What I suggested does not impact PCI compliance or any other
security parameters. Nor does it impact FDA compliance. All the
MODULE does is build an SQL string. When bound with other modules
to make an executable object there are no compliance issues.



If you create ILE programs that allow updating modules or service
programs then you have a behavior that needs changing. My opinion
is, and it is only my opinion, you should NEVER create a program
with Allow update. You are just asking for trouble in the ILE world.



If you have ulcer causing issues with pgmToBuildSQL make it a copy
module rather than a bound module.



Regards,



Gary





-----Original Message-----
From: rpg400-l-bounces@xxxxxxxxxxxx
[mailto:rpg400-l-bounces@xxxxxxxxxxxx] On Behalf Of Charles Wilt
Sent: Friday, July 29, 2011 10:35 AM
To: RPG programming on the IBM i / System i
Subject: Re: Reduce large amount of logicals in SUBFL pgm,take in
another direction



Very, very BAD IDEA!



Horrible in fact.



I hope you're not subject to PCI compliance rules, as the code you
posted pretty much guarantees an automatic failure.



Try googleing for "SQL injection"...



Charles



On Fri, Jul 29, 2011 at 1:16 PM, Monnier, Gary
<Gary.Monnier@xxxxxxxxx <mailto:Gary.Monnier@xxxxxxxxx> > wrote:

Sharon,

The process that builds your SQL statement can have parameters
passed

to it. One of these parameters can be a list of selections.

Something like this.

pgmToBuildSQL PR

nbrFieldsForWhereClause

arrayOfFieldsForWhereClause

arrayOfValuesForWhereClause

SQLStringBegin = 'Select * From yourfile ';

X = 1;

startPos = 1;

doW (X < = nbrFieldsForWhereClause);

whereValue = arrayOfFieldsForWhereClause(X) +

arrayOfValuesForWhereClause(X); //ValueFor can be = value, like

value, etc

valueLen = %len(%trimr(whereValue));

%subst(whereClause:startPos:valueLen) = whereValue;

startpos = startPos + valueLen + 1;

X = X + 1;

enddo;

Return;

--

This is the RPG programming on the IBM i / System i (RPG400-L)
mailing list To post a message email: RPG400-L@xxxxxxxxxxxx
<mailto:RPG400-L@xxxxxxxxxxxx> To subscribe, unsubscribe, or
change list options,

visit: http://lists.midrange.com/mailman/listinfo/rpg400-l
<http://lists.midrange.com/mailman/listinfo/rpg400-l>

or email: RPG400-L-request@xxxxxxxxxxxx
<mailto:RPG400-L-request@xxxxxxxxxxxx>

Before posting, please take a moment to review the archives at
http://archive.midrange.com/rpg400-l
<http://archive.midrange.com/rpg400-l> .



--
This is the RPG programming on the IBM i / System i (RPG400-L)
mailing list To post a message email: RPG400-L@xxxxxxxxxxxx To
subscribe, unsubscribe, or change list options,
visit: http://lists.midrange.com/mailman/listinfo/rpg400-l
or email: RPG400-L-request@xxxxxxxxxxxx Before posting, please take
a moment to review the archives at

http://archive.midrange.com/rpg400-l.

--
This is the RPG programming on the IBM i / System i (RPG400-L)
mailing list To post a message email: RPG400-L@xxxxxxxxxxxx To
subscribe, unsubscribe, or change list options,
visit: http://lists.midrange.com/mailman/listinfo/rpg400-l
or email: RPG400-L-request@xxxxxxxxxxxx Before posting, please take
a moment to review the archives at

http://archive.midrange.com/rpg400-l.

--
This is the RPG programming on the IBM i / System i (RPG400-L)
mailing list To post a message email: RPG400-L@xxxxxxxxxxxx To
subscribe, unsubscribe, or change list options,
visit: http://lists.midrange.com/mailman/listinfo/rpg400-l
or email: RPG400-L-request@xxxxxxxxxxxx Before posting, please take
a moment to review the archives at

http://archive.midrange.com/rpg400-l.

--
This is the RPG programming on the IBM i / System i (RPG400-L) mailing
list To post a message email: RPG400-L@xxxxxxxxxxxx To subscribe,
unsubscribe, or change list options,
visit: http://lists.midrange.com/mailman/listinfo/rpg400-l
or email: RPG400-L-request@xxxxxxxxxxxx Before posting, please take a
moment to review the archives at http://archive.midrange.com/rpg400-l.

--
This is the RPG programming on the IBM i / System i (RPG400-L) mailing
list To post a message email: RPG400-L@xxxxxxxxxxxx To subscribe,
unsubscribe, or change list options,
visit: http://lists.midrange.com/mailman/listinfo/rpg400-l
or email: RPG400-L-request@xxxxxxxxxxxx Before posting, please take a
moment to review the archives at http://archive.midrange.com/rpg400-l.

--
Rory Hewitt

http://www.linkedin.com/in/roryhewitt
--
This is the RPG programming on the IBM i / System i (RPG400-L) mailing list
To post a message email: RPG400-L@xxxxxxxxxxxx To subscribe, unsubscribe,
or change list options,
visit: http://lists.midrange.com/mailman/listinfo/rpg400-l
or email: RPG400-L-request@xxxxxxxxxxxx
Before posting, please take a moment to review the archives at
http://archive.midrange.com/rpg400-l.

--
This is the RPG programming on the IBM i / System i (RPG400-L) mailing list
To post a message email: RPG400-L@xxxxxxxxxxxx
To subscribe, unsubscribe, or change list options,
visit: http://lists.midrange.com/mailman/listinfo/rpg400-l
or email: RPG400-L-request@xxxxxxxxxxxx
Before posting, please take a moment to review the archives
at http://archive.midrange.com/rpg400-l.