Skip to content

add sending output format strings lab with python string format #618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 14, 2024
Merged

add sending output format strings lab with python string format #618

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions docs/labs/format-strings.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1">
<link rel="stylesheet" href="https://best.openssf.org/assets/css/style.css">
<link rel="stylesheet" href="checker.css">
<script src="js-yaml.min.js"></script>
<script src="checker.js"></script>
<link rel="license" href="https://creativecommons.org/licenses/by/4.0/">


<script id="expected0" type="plain/text">
return '{format_string}, {event.level}'.format(format_string=user_input, event=event)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still allows untrusted user input as a format string, and that's what we are telling them to avoid. Also, while "event=event" is Pythonic, it's really unclear to non-experts in Python, because they don't know which "event" is being used in the format string.

I'd expect something like this as an answer:

 return 'Event level = {event_level}'.format(event_level=event.level)

</script>

<!-- Full pattern of correct answer -->
<script id="correct0" type="plain/text">
\s*return\s'\{format_string\},\s\{event\.level\}'\.format\(format_string=user_input,\sevent=event\)\s*
</script>

<script id="info" type="application/yaml">
---
- absent: "user_input"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing this line after ---:

hints:

We support several different keys in the info, "hints" is one of them.

Once you do, you'll find a lot of the tests fail. You'll need the tests to pass :-).

text: Make sure the user_input is included in the replacement fields passed to the format function
examples:
- - "'some format'.format(user_input=user_input)"
- - "'some format'.format(user_input)"
- absent: "event"
text: Make sure the event is included in the replacement fields passed to the format function
examples:
- - "'some format'.format(user_input=user_input, event=event)"
- - "'some format'.format(user_input, event)"
- absent: \{(0|format_string)\}
text: Make sure the braces for the format_string replacement field are in the format string
examples:
- - "'{event}'.format(format_string=user_input)"
- - "'{0}.format(user_input)"
- absent: \{(1|event)\}
text: Make sure the braces for the event replacement field are in the format string
examples:
- - "'{format_string}, {event.level}'.format(format_string=user_input, event=event)"
- - "'{0}, {1.level}'.format(user_input, event)"
- absent: \{(1\.level|event\.level)\}
text: Make sure the level attribute of the event replacement field is in the format string
examples:
- - "'{format_string}, {event.level}'.format(format_string=user_input, event=event)"
- - "'{0}, {1.level}'.format(user_input, event)"
# debug: true
</script>
</head>
<body>
<!-- For GitHub Pages formatting: -->
<div class="container-lg px-3 my-5 markdown-body">
<h1>Lab Exercise Format Strings and Templates</h1>
<p>
This is a lab exercise on developing secure software.
For more information, see the <a href="introduction.html" target="_blank">introduction to
the labs</a>.

<p>
<h2>Task</h2>
<p>
<b>Practice using string templates in a secure way.</b>

<p>
<h2>Background</h2>
<p>
In this exercise, we'll adjust our string formatting so that it doesn't allow a user to control
the <a href="https://docs.python.org/3/tutorial/inputoutput.html#the-string-format-method"><tt>
format string</tt></a>. If a user can control the <tt>format string</tt> they can access
variables which they shouldn't. Particularly if those variable's values can be returned to the user
as output, it could lead to information disclosure beyond what was intended by the developer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add something like this:

In some cases the results can be even more serious, depending on the situation and programming language used. If a similar problem happened in C's printf function, it would often enable an attacker to completely control the program.


<p>
<h2>Task Information</h2>
<p>

<p>
Please change the code below so the string formatting cannot disclose arbitrary
program values. The server-side program is written in Python and allows a user to specify a
<tt>format string</tt> to control the output format of an event.

<p>
You could adjust the program so that it only formats the event, and does not include any user input,
However it is considered safe to include user input in the output as long as they cannot control
the <tt>format string</tt> itself.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's WAY too strong a claim. Just say that allowing arbitrary user control of format strings is dangerous.


<p>
Adjust the value returned by the <tt>format_event</tt> function so the the user controlled
<tt>user_input</tt> variable is only used as a
<a href="https://docs.python.org/3/library/string.html#format-string-syntax"><tt>replacement field</tt></a>
and is not used as the <tt>format string</tt>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh? This doesn't look secure.

This still allows for user control of the field to be shown. Depending on the object being passed to format, that can still leak secret data that isn't supposed to be shared. It might be okay in context, but I think the lab should focus on the "key point" which is that format strings can be dangerous, and if you don't HAVE to have them controlled by the user, don't.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Be Careful with Python's New-Style String Format article uses "the somewhat undocumented internals to change the behavior" in an attempt to sandbox things. That's a complex and dangerous way to resolve security issues. Sure, sometimes you must do something quirky like that, but there's a simple one: don't do it. If you only get format strings from trusted sources, the problem disappears. The article mentions translators, but usually translators are a small number of trusted people, very different from accepting arbitrary inputs from attackers.


<p>
Use the “hint” and “give up” buttons if necessary.

<p>
<h2>Interactive Lab (<span id="grade"></span>)</h2>
<p>
<form id="lab">
<pre><code
> # Application configuration which should be kept secret from a user
CONFIG = {
'SECRET_KEY': 'super secret key'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't do that! That would be a secret key embedded in source code, which is itself a vulnerability.

}

# A event object with a single attribute used by the malicious format string to gain access to the
# secret application configuration below
class Event(object):
def __init__(self, level):
self.level = level

def format_event(user_input, event):
<input id="attempt0" type="text" size="60" spellcheck="false" value=" return user_input.format(event=event)">

event = Event('level')
format_event('{event.__init__.__globals__[CONFIG][SECRET_KEY]}', event)
</code></pre>
<button type="button" class="hintButton">Hint</button>
<button type="button" class="resetButton">Reset</button>
<button type="button" class="giveUpButton">Give up</button>
<br><br>
<p>
<i>This lab was developed by Jason Shepherd at
<a href="https://access.redhat.com"
>Red Hat</a>.</i> with an modified version of the example code from Armin Ronacher's
<a href="https://lucumr.pocoo.org/2016/12/29/careful-with-str-format/">Be Careful with Python's New-Style String Format</a> article.
<br><br>
<p id="correctStamp" class="small">
<textarea id="debugData" class="displayNone" rows="20" cols="65" readonly>
</textarea>
</form>
</div><!-- End GitHub pages formatting -->
</body>
</html>
Loading