11///////////////////////////////////////////////////
22/// ///
3- /// Vulnerability was in line 49 of code.h ///
4- /// Fix can be found in line 47 below ///
3+ /// Vulnerability was in line 84 of code.h ///
4+ /// Fix can be found in line 83 below ///
55/// ///
66///////////////////////////////////////////////////
77
1313
1414#define MAX_USERNAME_LEN 39
1515#define SETTINGS_COUNT 10
16- int userid_next = 1 ;
16+ #define MAX_USERS 100
17+ #define INVALID_USER_ID -1
1718
19+ /*
20+ To keep things simple, both private (implementation specific) and public (API) parts of
21+ the application have been bundled inside this header file. In reality, you would
22+ only keep the API here. That being said, assume that the private sections would not be
23+ known to casual users of this module.
24+ */
25+
26+ // Internal counter of user accounts.
27+ int userid_next = 0 ;
28+
29+ // This whole structure is purely an implementation detail and is supposed to be
30+ // unknown for normal users.
1831typedef struct {
1932 bool isAdmin ;
2033 long userid ;
2134 char username [MAX_USERNAME_LEN + 1 ];
2235 long setting [SETTINGS_COUNT ];
2336} user_account ;
2437
25- user_account * create_user_account (bool isAdmin , const char * username ) {
26- user_account * ua ;
27- if (strlen (username ) > MAX_USERNAME_LEN )
28- return NULL ;
38+ // Simulates an internal store of active user accounts.
39+ user_account * accounts [MAX_USERS ];
40+
41+ // The signatures of the next 4 functions together with previously introduced constants (see #DEFINEs)
42+ // constitute the API of this module.
43+
44+ // Creates a new user account and returns it's unique identifier.
45+ int create_user_account (bool isAdmin , const char * username ) {
46+ if (userid_next >= MAX_USERS ) {
47+ fprintf (stderr , "the maximum number of users have been exceeded" );
48+ return INVALID_USER_ID ;
49+ }
50+
51+ user_account * ua ;
52+ if (strlen (username ) > MAX_USERNAME_LEN ) {
53+ fprintf (stderr , "the username is too long" );
54+ return INVALID_USER_ID ;
55+ }
2956 ua = malloc (sizeof (user_account ));
30- if (NULL == ua ) {
57+ if (ua == NULL ) {
3158 fprintf (stderr , "malloc failed to allocate memory" );
32- return NULL ;
59+ return INVALID_USER_ID ;
3360 }
3461 ua -> isAdmin = isAdmin ;
3562 ua -> userid = userid_next ++ ;
3663 strcpy (ua -> username , username );
3764 memset (& ua -> setting , 0 , sizeof ua -> setting );
38- return ua ;
65+ accounts [userid_next ] = ua ;
66+ return userid_next ++ ;
3967}
4068
41- bool update_setting (user_account * ua , const char * index , const char * value ) {
69+ // Updates the matching setting for the specified user and returns the status of the operation.
70+ // A setting is some arbitrary string associated with an index as a key.
71+ bool update_setting (int user_id , const char * index , const char * value ) {
72+ if (user_id < 0 || user_id >= MAX_USERS )
73+ return false;
74+
4275 char * endptr ;
4376 long i , v ;
4477 i = strtol (index , & endptr , 10 );
4578 if (* endptr )
4679 return false;
47- if (i < 0 || i >= SETTINGS_COUNT ) // FIX
48- return false;
80+
4981 v = strtol (value , & endptr , 10 );
50- if (* endptr )
82+ // FIX: Checking for negative index values, too!
83+ if (* endptr || i < 0 || i >= SETTINGS_COUNT )
5184 return false;
52- ua -> setting [i ] = v ;
85+ accounts [ user_id ] -> setting [i ] = v ;
5386 return true;
5487}
5588
89+ // Returns whether the specified user is an admin or not.
90+ bool is_admin (int user_id ) {
91+ if (user_id < 0 || user_id >= MAX_USERS ) {
92+ fprintf (stderr , "invalid user id" );
93+ return false;
94+ }
95+ return accounts [user_id ]-> isAdmin ;
96+ }
97+
98+ // Returns the username of the specified user.
99+ const char * username (int user_id ) {
100+ // A better approach would be to signal an error.
101+ if (user_id < 0 || user_id >= MAX_USERS ) {
102+ fprintf (stderr , "invalid user id" );
103+ return NULL ;
104+ }
105+ return accounts [user_id ]-> username ;
106+ }
107+
56108/*
57- Buffer Overflow Vulnerability
109+ Security through Obscurity Abuse Vulnerability
110+ --------------------------------------------
111+ You may read about the concept of security through obscurity here:
112+ https://en.wikipedia.org/wiki/Security_through_obscurity
113+
114+ In code.h the user_account structure is supposed to be an implementation
115+ detail not handed over to the user. Otherwise, they could easily modify the
116+ structure and change the isAdmin flag to true, thus gaining admin privileges.
117+
118+ Nonetheless, as this example illustrates, security through obscurity alone is not enough
119+ to secure your system. The attacker can easily reverse engineer the code and
120+ find the vulnerability. This is exposed in hack.c (see below).
58121
59- In hack.c, an attacker escalated privileges and became an admin by abusing
60- the fact that the code wasn't checking for negative index values.
122+ Buffer Overflow Vulnerability
123+ ----------------------------
124+ In hack.c, an attacker escalated privileges and became an admin by abusing
125+ the fact that the code wasn't checking for negative index values.
61126
62- Negative indexing here caused an unauthorized write to memory and affected a
63- flag, changing a non-admin user to admin.
127+ Negative indexing here caused an unauthorized write to memory and affected a
128+ flag, changing a non-admin user to admin.
64129*/
0 commit comments